Created
July 14, 2014 16:07
-
-
Save CarlosGabaldon/20dd8067af1bd9b5dbfe to your computer and use it in GitHub Desktop.
Code Review for Yann Marquet
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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