A code review checklist is the first step in creating a repeatable and reliable code review process for your team.
Code review is an opportunity to catch problems early, learn from other developers, and see past our own blind spots. While each of us as developers has our own approach to code review, standardizing on a code review checklist helps make sure no part of the review gets missed while leaving plenty of flexibility.
Here's some advice on creating a checklist that will work for your team.
Code review checklist or code review template?
First up, should you create a code review checklist or a code review template?
Not sure of the difference? Think of it this way—a template demands answers, whereas a checklist poses questions.
Human creativity is essential to software development. Stifling that creativity in the hope of avoiding the unknown rarely results in better code. Code review can be just as much a part of the creative process as the initial coding itself. Set too rigid a template and the creativity gets lost. Suggest a shape for the code review conversation and you provide guide rails rather than restraints.
What should be in your code review checklist?
Your code review checklist is just one part of creating a set of code review best practices for your team. You might find it helpful to agree on standards for how long a code review should take, for example. Similarly, everyone involved in a code review should know what to do next when the review identifies a defect.
But without a robust code review checklist, there's a risk that any other code review process is working with imperfect material. So let's look at the broad areas your code review should cover.
Does it do what it is meant to?
This is what you might call the "table stakes" question and it comes in two parts:
- Does the code set out to implement the functionality requested in the spec or ticket?
- Does the code succeed in implementing that functionality?
In other words, did the original developer understand the problem correctly, and did they solve that problem?
Unless something isn't working in your team, then the original developer has already thought about this and verified that their code meets the requirements. But it's easy to get too close to our code and, in doing so, fail to see what might be obvious to someone with fresh eyes.
Use this part of the code review checklist to prompt reviewers to think about the non-obvious. What are the edge cases? Is this solution the most effective?
Is this the simplest solution?
In this part of the code review, the reviewer should be able to answer whether this code solves the problem at hand or, instead, solves a more generic version of the problem that isn't relevant right now. Similarly, does the code under review use the tools and techniques appropriate to the problem at hand or does it suffer from premature optimization?
It's easy to over-engineer. We see a problem and, for many of us, there's a certain excitement in creating ever more abstract solutions that solve a whole class of problems rather than only the specific one set out in the spec. Similarly, it's exciting to learn and use new tech when, actually, something simpler and rather more boring would be more appropriate.
Does it serve the user?
Later in the SDLC, there are other ways of testing this, such as user acceptance testing. However, it's worth spending some of the review considering whether this is the best solution and implementation for the end user(s).
Does this code duplicate something that already exists?
In larger codebases, it's hard for everyone to know every last bit of functionality. Does this pull request introduce functionality that already exists? Similarly, would it be better to pull in a third-party library, REST API, or other product rather than re-inventing something that is a solved problem elsewhere?
Does the code serve future developers?
Whether it's yourself, the person whose code you're reviewing, or someone else entirely, a developer will need to use or change this code at some time in the future.
Is the code readable? Is it reusable in different contexts? Does it meet the team or repo's style guidelines? Are things named in a way that makes their function and usage obvious? Do comments provide enough context without going off topic?
Perhaps most importantly, does this PR take care of updating developer documentation outside of comments? If not, does the developer have a plan to take care of that?
Are there tests? Do they cover the right things?
This code will need new tests of its own and might also require changes to existing tests.
Pretty much every part of the code review checklist also applies to the tests. Are they simple enough? Do they fail when the code breaks? Is there scope for false positives? Is the scope of each test appropriate?
Is the code secure?
This section of your code review should be supplemented by static application security testing later in your SDLC. However, a human reviewer should ask whether this code expands the potential attack surface of your codebase. The specifics will depend on your team's needs, but at the very least reviewers should determine whether new dependencies are trustworthy and check for common security pitfalls.
Is this reliable, performant, and scalable?
A code review isn't the only place your process should ask these questions. However, your code review checklist should prompt the reviewer to look at whether the code is optimized in a way that matches its intended usage patterns. Similarly, is it plugged into your team's instrumentation to enable performance monitoring and failure reporting?
Code review is a conversation between humans
Code review is about people as much as it is about code. While it's important to highlight things that could be improved, make sure that your code review checklist prompts the reviewer to offer encouragement and to praise their colleague for things done well.
Whatever the specific role of code review in your team, a code review checklist provides a light structure for code reviews that puts that human conversation at the center.