Skip to content

Grading/Feedback PR [DO NOT MERGE]#59

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

Grading/Feedback PR [DO NOT MERGE]#59
martypdx wants to merge 2 commits intoTEAMBENTO:masterfrom
martypdx:master

Conversation

@martypdx
Copy link
Copy Markdown

@martypdx martypdx commented Jul 3, 2018

Nice React app. A few issues, but pretty solid overall. Your server routes did a good job giving the app what it needed, so actions were very clean and simple!

  • It was a bit of a struggle to get user and profile info loaded. Try and narrow this down to single place like App where you know it will always run.
  • Property naming was hard to follow at times.
  • Don't repeat hard-coded values over and over (categories)
  • When you are calling multiple action creators in a component, good sign that you need higher-level action creator that encapsulates work to be done.
  • Use selectors to derive data, not state
  • Use camelCase, avoid camelcase
  • Categories <select> is repeated 6 times! Create a common component!

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