Skip to content

Grading PR - Comments and Feedback [DO NOT MERGE]#27

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

Grading PR - Comments and Feedback [DO NOT MERGE]#27
martypdx wants to merge 2 commits intocolordiary:masterfrom
martypdx:master

Conversation

@martypdx
Copy link
Copy Markdown

@martypdx martypdx commented Apr 7, 2017

  • nice date formatting functions, and way to put them in their own file :)
  • Your components could probably be organized into folders, usually by primary route, and then general shared components
  • Refactor small snippets of repetitive html template to Components
  • Generally really good javascript modularization
  • Don't leave boilerplate props in your component functions when they're not used.
  • SignIn and SignUp have a lot of repetitive code between them
  • Some of the components like UserMoodDay and UserMoodSelector didn't get cleaned up and are really hard to read.
  • Consider not putting handler functions inline in the render function when they make it much harder to read because you can't see the template when there are so many lines of js.
  • no tests

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