Skip to content

Instantly share code, notes, and snippets.

@CarlosGabaldon

CarlosGabaldon/pr_review_yann_marquet

Created July 14, 2014 16:07
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/20dd8067af1bd9b5dbfe to your computer and use it in GitHub Desktop.
Save CarlosGabaldon/20dd8067af1bd9b5dbfe to your computer and use it in GitHub Desktop.
Download ZIP
Code Review for Yann Marquet
Raw
pr_review_yann_marquet
Story 1
- Solved the problem in common expected way with includes
- Would have liked to see a few commit notes on the change, expaling any assumptions and/or possible future enhancements
Story 2
- Liked the addtion of the ThemePathResovler to abstract the paths
- + for unit test
Story 3
- Used Timecop, which is the most common way to solve this time issue
Story 4
- Clean approach
- + for unit tests
Overall
- Clean implementation, most of the problems were solved with the most common expected ways
- Easy to follow PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment