- Each submission has a goal, so as a reviewer your aim is to:
- Answer the question: 'Does this work show evidence of progress towards the goal?'
- Provide reasons you think it does, or feedback what can be improved upon.
- Try to review to the standard that the code is at: no need to make line edits if there are a lot of style violations.
- Try to leave at most 3 specific overall targets for the developer to focus on
- Don't spend too long reviewing the code. 15 mins is probably too much.
Here's a list of things you might want to look at when reviewing.
- Testing against the specs/acceptance criteria if given
- Passing tests, no flaky tests (mocking time, mocking randomness, etc..)
- Coverage > 95%
- Feature test
- Is it the first test they wrote (BDD)?
- Unit tests
- Are they isolated?
- Are mocks used properly? Dependency injection
- Are third party libraries + language internals mocked?
- Are unhappy paths and edge cases tested?
- Is there a linter? Does it pass?
- Clear class/method/variable/test names
- Names are not abbreviated
- Code is easy to understand (speaks for itself)
- Single Responsibility
- Not heavily coupled
- No repetition of code
- Only outputs/prints in one clearly identified method (View in MVC)
- Where are objects instantiated?
- Is the code inefficient (slow or memory intensive)?
- Consistent style of programming (Is the code solving the same problem in the same way in different classes?)
- Is one class is significantly longer than all the others
- Is there a clear name for the repo?
- Is there a README?
- README starts with a clear explanation of the programme for the casual reader/recruiter.
- No Jargon
- README describes how to install your programme and its dependencies
- README describes how to use it - maybe includes a screenshot
- README describes their approach and strategy for solving the problem
- README has reasonable spelling and grammar
- Is there a LICENCE? On purpose?
- Is the repo organised?
- Clear commit messages
- Tests with the code committed at the same time