Skip to content

Grading/feedback PR [DO NOT MERGE]#37

Open
martypdx wants to merge 2 commits into
wtfturtle:masterfrom
martypdx:master
Open

Grading/feedback PR [DO NOT MERGE]#37
martypdx wants to merge 2 commits into
wtfturtle:masterfrom
martypdx:master

Conversation

@martypdx
Copy link
Copy Markdown

  • No README or other project info
  • Removed unused files and code (i.e. LoginForm and PrivateRoute)
  • Generally good project structure and organization! Especially breaking down App into Header, Footer, etc.
  • Add and AddDetail look identical, as do Remove and RemoveDetail. Either poor team coordination or poor code management.
  • Several components used connect but did not use any of the info.
  • AFAICT Rating and Thumbs components didn't do anything persistent :(
  • Flow of search not handled well. You put search logic in your presentation, belongs in actions
  • The purpose of destructuring in render methods is to organize and make clear which props you are using. Repeating the same destructuring over and over defeats the purpose.
  • Tenuous grasp on redux using async with reducers. Many lurking issues.
  • Do more work in foursquare service to normalize and clean up data, don't wait until the data is in presentation components

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