Last week, I wrote about the practice of doing code reviews, and how asking questions helps achieve a greater understanding of the subject at hand (in this case, the code). In order to help the team follow consistent code review practices, I created a checklist of questions to ask oneself while reviewing code.
After a few revisions, this is the list that we currently use, minus some sections about technologies specific to our project. In case you want to use it as the base for your own checklist, you can fill those in yourself. I plan on doing an in-depth revision of the items in it on a future post.
Overall checks
- Read the PR title and description, to understand scope
- Verify that the list of modified files matches expectations
Check tests
- Read tests before looking at the implementation code
- Does every implementation file have an accompanying test?
- Does every test have an assertion?
- Do the test descriptions match the assertions in the tests?
- Can the test descriptions be read in isolation while remaining clear?
- Do tests have a clear narrative order?
- Do tests cover all the expected functionality?
- Are tests covering every edge case you can think of?
- Is the data used in the tests properly mimicking the structure of real data?
- WARNING Is there excessive mocking in a test?
Check high-level implementation code
- Are all variable names nouns?
- Are all function names verbs?
- Are names meaningful?
- Are names consistent with other uses in preexisting code?
- Do module names match the abstraction they represent?
- Are abstractions at the right level?
- Are patterns used in preexisting code respected?
Check implementation details
- Does the implementation match the expectations from reading the tests?
- Are all the code paths covered by the tests?
- Are functions short?
- Do functions have a single clear responsibility?
- Is the exposed API minimal?