Skip to content

logan lab 11-12#11

Open
loganlsr wants to merge 9 commits intocodefellows-seattle-javascript-401d10:masterfrom
loganlsr:logan-lab-11-12
Open

logan lab 11-12#11
loganlsr wants to merge 9 commits intocodefellows-seattle-javascript-401d10:masterfrom
loganlsr:logan-lab-11-12

Conversation

@loganlsr
Copy link
Copy Markdown

No description provided.

Raziyehbazargan pushed a commit to Raziyehbazargan/lab-11-12-express-api that referenced this pull request Sep 27, 2016
@loganlsr loganlsr changed the title server set up, construtor made, lib set up, still need to write tests logan lab 11-12 Sep 28, 2016
@@ -0,0 +1 @@
{"id":"245efb70-84f4-11e6-9ac7-f5fcfad695e3","type":"example","color":"colorexample","size":"examplesize"} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr no need to push up your JSON data files - add these to your gitignore to avoid overpopulating the repo

Comment thread gulpfile.js
@@ -0,0 +1,19 @@
'use strict';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr gulpfile looks good

Comment thread lib/cors-middleware.js
@@ -0,0 +1,7 @@
'use strict';
Copy link
Copy Markdown

@bnates bnates Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr nice inclusion of the cors module for allowing all requests

Comment thread lib/error-middleware.js
err = createError(500, err.message);
res.status(err.status).send(err.name);
next();
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr error middleware looks good

Comment thread lib/storage.js
const debug = require('debug')('note:storage');
const fs = Promise.promisifyAll(require('fs'), {suffix: 'Prom'});

const mkdirp = Promise.promisifyAll(require('mkdirp'));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr good use of promises in conjuction with the mkdirp module

Comment thread lib/storage.js
return fs.readdirProm(`${__dirname}/../data/${schemaName}`)
.then( filenames => filenames.map(name => name.split('.json')[0]))
.catch(err => Promise.reject(createError(404, err.message)));
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr storage module looks good

Comment thread test/apple-test.js
type: 'example',
color: 'colorexample',
size: 'examplesize'
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr good mock data

Comment thread test/apple-test.js
});

describe('testing POST requests to /api/apple', function(){
describe('with a valid body', function(){
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr no need for a space here - that said, your after block looks good

Comment thread test/apple-test.js
});
});
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loganlsr tests look good - keep it up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants