Code reviews are an essential part of software development. To be good at the craft, we must be able to review the craftsmanship of others. The more books you read, the better you write; the more code you review, the better you code. The following 12-item code review checklist is a breadth-first traversal; each item is presented at an extremely high-level and deserves more of your attention.
Complete Code Review Checklist
#1—Does the code work?
- Take your time and understand the presented changes. This is a basic pre-requisite before you review any code.
- Don’t assume the code works out of the box.
- If you think you found a bug, there’s probably a bug.
#2—Are there abuses of memory?
- If appropriate, does the code release all the memory it originally allocated?
- Does any operation potentially use a dangerous amount of memory? Unbounded DB reads, mis-use of data structures, etc.
- Is the code caching too aggressively?
- Large memory usages should be called out and justified.
#3—Are there abuses of performance?
- Would these changes disrupt application performance?
- Extra I/O that increases the response times of a web service.
- Extra logging that slows down the runtime of a software simulation.
- Can any CPU cycles be saved by pushing some business logic down into the DB?
#4—Are there local concurrency issues?
- For multi-threaded applications, are the code changes thread-safe?
- Are multiple threads using non-thread-safe data structures?
- Are there any potential deadlocks?
#5—Are there distributed concurrency issues?
- Once you’re developing services within a distributed system, handling concurrency issue is a whole new ball game.
- Does the code orchestrate multiple network requests? If so, how does it handle error scenarios? Can the state of distributed data get into a bad state?
- If any code look like it’s attempting a distributed transaction? If so, pay extra attention to it.
- Distributed concurrency issues are deep issues and often need to be resolved in a more formal setting than code reviews.
#6—How risky is the change?
- If the change is risky, has the author added defensive measures to make sure something terrible doesn’t happen?
- Is monitoring and observability accounted for?
- If the code lies in a critical path, does it have to be?
- Have disaster scenarios been thought through? Downtime, corrupted data, lost money.
- How easily can these changes be turned off?
#7—Is the code review too large?
- Is the code review attempting to handle too many concerns?
- Does the code tell a story? With thoughtful story-telling, your code can be:
- Deployed logically, correctly, and safely
- Easy for your colleagues to understand
- Cleanly placed into the history books to be understood by future developers
#8—Does the code follow existing patterns?
- Does the code go against existing patterns?
- Are stylistic standards maintained? Variable naming, file length, comment style.
- Are coding patterns maintained? Exception handling, interface design, testing philosophy.
- If the author is going against existing patterns, and doesn’t seem to have a solid reason—say something.
#9—Does the code introduce new patterns?
- New features bring new code. Inevitably, new patterns will be introduced.
- New is not bad.
- If the author has introduced a new pattern, does it make sense for what he/she is trying to solve?
- Do the new patterns work alongside the old patterns?
#10—Can any language feature be utilized better?
- Is there something you know about the language that the author doesn’t?
- Are there any language constructs that aren’t utilized properly?
- Are there any locations where a language construct would make the implementation cleaner?
#11—Can any framework feature be utilized better?
- Exactly the same as above, except for frameworks—React, Rails, Spring, Django.
- Are there any framework constructs that aren’t utilized properly?
- Are there any locations where a framework construct would make the implementation cleaner?
#12—Is the code readable?
- At the end of the day, code is communication.
- Your teammates need to read it. You need to read it.
- Don’t be afraid to call out readability issues. Code is to be read by humans, not by computers.
- Be careful with unnecessary amounts of abstraction; this will cause unnecessary amounts of mental energy.
Bonus: Other Things To Keep In Mind
- This code review checklist is not relevant for every code review.
- You’ll have to look over large code review multiple times over.
- Code reviews are a good place to learn and ask questions.
- When commenting, be clear about what needs to be addressed now versus what can happen in a follow-up.
- Document contributing guidelines and code design principles. It’s tiring to repeat the same conversation in every review.
- Be reasonably prompt; don’t take two weeks to review your teammate’s code.
- Don’t be a jerk.