Skip to content

Grading PR with comments and feedback [DO NOT MERGE]#54

Open
martypdx wants to merge 2 commits intodbSimsPDX:masterfrom
martypdx:master
Open

Grading PR with comments and feedback [DO NOT MERGE]#54
martypdx wants to merge 2 commits intodbSimsPDX:masterfrom
martypdx:master

Conversation

@martypdx
Copy link
Copy Markdown

@martypdx martypdx commented Mar 9, 2017

Over very ambitious project with nice functionality. Biggest thing is keeping the code refactored and clean to make it easier to work with.

  • A lot of good effort in testing!
  • Promise chaining went off the rails in the longer, more complicated uses. Need to refactor and rework to not create "big balls of mud"
  • clean up: remove console.log, unused imports, dead files.
  • Don't use variables to pass state between steps in promise chain, unless you need to preserve data from a prior step for use in a later step. That technique is a "last resort", not a go-to.
  • For a data server, avoid string based requests and responses (wrap in an object)
  • Use .lean() and .select() on your GETs
  • Move logic to model when it makes sense (look for long code in routes)

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.

1 participant