Skip to content

Instantly share code, notes, and snippets.

@CarlosGabaldon

CarlosGabaldon/pr_review

Created July 9, 2014 19:06
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save CarlosGabaldon/44d5ec89c3db026eae46 to your computer and use it in GitHub Desktop.
Save CarlosGabaldon/44d5ec89c3db026eae46 to your computer and use it in GitHub Desktop.
Download ZIP
Review of Stephen Best Deal Site PR
Raw
pr_review
- Very clean implementation
- Easy to follow PR
- Liked the abstaction of adding the eager loading to the Deal model behind the very descriptive all_with_advertisers class method
- Liked that the code was profiled to track before and after perfomance improvements
- Very nice that unit test and migration was added for the DRY'ing up of all the city specific entertainment themes
- Did not use TimeCop for addressing the bad time test, but the choosen method was clean and effective
- The publisher importer solution was effective enough for the code exericse, liked that there were notes on it's limitations, unit test, and how it could be improved.
- Extra points for thinking about the Law of Demeter and how to better lessen coupling.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment