Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments #121

Open
g-sam opened this issue Dec 1, 2016 · 5 comments
Open

Comments #121

g-sam opened this issue Dec 1, 2016 · 5 comments

Comments

@g-sam
Copy link

g-sam commented Dec 1, 2016

Nice work. Well done for getting it hosted. Some comments:

  • Should be POST. See this.
  • Do this inside callback. And here.
  • As you are mutating objs, use forEach instead of map here. And here.
  • In a few places you are making two db queries where one (albeit more complicated query) would suffice.
  • In dbrequests folder you are using three different file naming conventions. You should anyway not use capital letters in file names.
  • I first met this resource on a cruise sheep in the baltic.
@Jwhiles
Copy link
Member

Jwhiles commented Dec 2, 2016

Thanks for the suggestions,

could you clarify the second point? I'm not totally sure what you mean.

@Jwhiles
Copy link
Member

Jwhiles commented Dec 2, 2016

I will say, it's not our fault if our users can't spell ;)

@g-sam
Copy link
Author

g-sam commented Dec 2, 2016

re the second point, I mean you should reply inside the callback that fires after the database request has returned. Otherwise you may redirect (so the user thinks everything is fine), then suddenly an error is thrown when the db returns.

@njsfield
Copy link
Contributor

njsfield commented Dec 2, 2016

For point 5;

  • In dbrequests folder you are using three different file naming conventions. You should anyway not use capital letters in file names.

Could you recommend any resources on learning good naming conventions?

@g-sam
Copy link
Author

g-sam commented Dec 2, 2016

Consistency is the key. It doesn't matter what the convention is. The only thing to be aware of is that capital letters will be ignored by some operating systems so should be avoided.

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

No branches or pull requests

3 participants