What You Need in a Code Review Checklist (& What You Don’t)

Share This Post

I’ve been able to successfully implement a code review process in teams a couple of times, and the opportunity to see the code improving before my eyes brought me much joy. However, code reviews won’t improve code quality if you try to improvise; you need a standardized, repeatable process, and that’s what this post is all about. In it, I’ll provide you with a list of items to add to your code review checklist.

The list is deliberately short. When the code review process gets too long, it’s usually a sign of trouble. Generally speaking, you want your pull request review time to be relatively low.

As a bonus, I’ll also cover a few things that you shouldn’t add to your code review process. Let’s get started.

Table of Contents

Things to Add to Your Code Review Checklist

Let’s start with some of the items I think are indispensable in a code review checklist.

code review pull quote

Try to Identify Obvious Bugs

This is priority number 1 of a code review: Check if the code is working.

Even great engineers write code that has defects. Often, those defects are quite silly: an off-by-one error, a misspelled variable, parameters passed in the wrong order to a method, and so on.

Code reviews offer a great opportunity to catch these defects because, while the author often misses them, a different person with a fresh pair of eyes will spot them more easily.

Look for Possible Security Issues

When reviewing code, try to look for possible security issues that could be exploited. Sure, your CI pipeline should make use of plugins and other tools that automatically check for many types of vulnerabilities. But when reviewing code, you might find problems the author was too sleep-deprived to notice—e.g. controller actions without protection against CSRF or SQL queries that concatenate user input and become vulnerable to injections.

Tired Denzel Washington GIF by Bounce - Find & Share on GIPHY
Even great developers can have a lapse in attention and make mistakes.

Look for “Clever” Code

Code readability is another vital area you should look into when reviewing code. First, remember that readability is, to a certain extent, subjective. Take a look at the following piece of code:

{ showList && <ProductList />}

This is a common idiom in JavaScript/React, and it allows rendering a component conditionally. However, a C# or Java developer might find that unreadable, and prefer something like this:

if (showList) {
    let return =  <ProductList />;
}

In this example, I prefer the first snippet, even though I’m mostly a backend engineer myself. But this is just my opinion. The point is that code readability is a subjective judgement call that must take into account different coding styles and what’s idiomatic for each language.

What you should do when evaluating readability is to keep an eye out for code that tries too hard to be clever. That includes code that puts brevity above clarity, using somewhat obscure language constructs in order to save a few keystrokes.

Check for Code Duplication

When reviewing code, you’ll often spot some low-hanging fruit regarding code duplication. Maybe the author planned to extract the duplication to a dedicated method but then forgot to do it. Sometimes there’s a nice abstraction just lurking around and the author can find it with the right push. Maybe the functionality already exists elsewhere and can be reused.

code review pull quote

Check Whether Names Are Descriptive Enough

Naming is one of the hardest things in software engineering, but that doesn’t mean we should give up. When performing a review, look for opportunities to improve the names of variables, constants, class fields and properties, methods, classes, and so on.

Sometimes, the code is trying to be clever with the naming by picking fancier names than what’s needed. For instance, I often see people creating interfaces to abstract access to the system clock so they can unit test code that makes use of time. Fair enough, but then they name this abstraction “ITimeProvider,” or something like that, when in the real world there’s already an object that provides the time: a clock!

With the help of a patient, creative, and empathetic reviewer, engineers can certainly find names that are creative yet simple and adhere to the language used by the rest of business.

Look for Possible Performance Improvements

Again, you should have automatic checks and production monitoring to detect performance issues. However, that doesn’t mean you shouldn’t look for opportunities to increase performance while doing code reviews. I’m thinking about easy finds like:

  • an expensive operation inside a loop
  • excessive allocations of objects
  • inefficient string concatenations
  • inefficient logging

You’ll often spot seemingly simple problems during the review that will go a long way in improving the code and its performance.

Check the Presence and Quality of Tests

Automated tests—including but not limited to unit tests—are code and, as such, you should also review them. When reviewing unit tests, check for:

  • The presence of tests: Did the author create tests for their change?
  • The quality of tests: Do the tests created seem to effectively exercise the system under test? Do they follow agreed-upon best practices?
  • Readability: Remember tests are also documentation. They should be simple and easy to understand.
  • Naming: Are the tests named according to the team’s convention? Is it easy to understand what they’re about?

Explain Your Changes

When you feel that the author of a delta you are reviewing wrote less-than-perfect code because of a lack of knowledge, it’s important to take the time to educate them. One of the major benefits of code reviews is that they spread knowledge throughout the team and help less-experienced developers level up their skills. Although each team member will have their areas of expertise, the team will perform better if everyone has a comprehensive understanding of the codebase.

It can be hard to take the time to explain something, but think of it as a long-term investment that will improve the quality of the code to be reviewed, which will make reviews easier to conduct in the future. Better than catching bugs early is making sure that the bugs don’t appear at all!

Increasing knowledge among the team will also improve the overall quality of the codebase, resulting in reduced development costs. In this way, investing in learning is like preemptively paying down technical debt.

Optional: Code Documentation

I added this as optional because I think it really depends on the conventions and rules of each team. I personally am a big fan of Docblocs. That is code documentation such as XML comments in C#, Javadoc for Java, and phpDoc for PHP. By keeping comments in the code itself, this makes updating the documentation much easier and therefore more likely to happen.

During the build process, these comments can be used to generate documentation that is then uploaded to a server where they can be accessed by developers on the team. Such a style of documentation is particularly useful for teams creating software that’s used by third parties, such as libraries. Your mileage may vary, and that’s why I consider this optional.

Things to Remove from Your Code Review Checklist

As a bonus, let’s quickly go through some things that, in my opinion, should not be a part of your code review process.

Cosmetic Concerns

You shouldn’t be wasting your time fixing lots of cosmetic/aesthetic issues during your code review. These should be taken care of by an automated tool like a linter or static analyzer. You can configure linters to enforce one of the established coding style guides (AirBnB’s is a popular one) or to enforce your company’s style.

What do I mean by cosmetics? Things like:

  • whether to use tabs or spaces
  • the position of opening brackets in code blocks
  • whether to leave a space after if, for, and other keywords
  • whether to leave a blank line in certain situations
  • snake_case vs camelCase vs PascalCase

Testing

In my view, reviewers shouldn’t test changes when performing a code review. When I say test changes, I mean pulling the branch to their machines, executing the application, and manually going to the changes in order to see if everything’s working properly.

If the reviewer feels the need to test a pull request in this way, this means that either the pull request is too large and complex, or the team lacks a proper testing strategy. Or probably both.

Anything That Can Be Automated

As a general rule, avoid doing anything in a code review that can be automated. For instance, I just talked about how you should review unit tests when performing reviews. Sure, but that review should not include verifying whether the tests cover all possible branches of the code. That would be too time-consuming anyway, but there’s an automated way to check that: it’s called branch coverage.

Another example would be compliance or adherence to the tech stack agreed upon. There are ways to automate that, and it should be automated.

Another piece of the code review process that should be automated are notifications. Engineers shouldn’t be taking valuable time to ping another about code reviews. Instead, use a tool like LinearB’s WorkerB automation bot which will handle notifying people about a review that has been assigned to them. It’ll even let them know if they’ve forgotten about a review – no more awkward Slack messages that start with, “Hey, just wanted to check in about this…”

PR left on read? Not anymore. Schedule a demo with LinearB.
Promote your pull requests to merge 10x faster. Book a demo today!

Review Your Team’s Code for Fun and Profit

Code reviews are invaluable for the health of a codebase. During reviews, you can detect bugs, find opportunities to improve security and performance, and much more. But to be able to reap the benefits of code review, your team needs to agree upon a standardized process. Improvisation and lack of consistency during reviews harm the team’s morale. If my pull request gets rejected by a reviewer because I followed the recommendations from a previous reviewer, I’ll conclude the whole process is arbitrary and useless.

code review pull quote

That’s why I shared this list of items I think you should include in your checklist. If you don’t have a checklist, create one ASAP. Your codebase and team will thank you.

Improve your engineering organization at every level with LinearB
Want to improve your engineering processes at every level? Get started with LinearB today!

More To Explore

Never miss a post

Join our community of data-driven dev leaders

Each week we share stories and advice from engineering and product leaders striving to be better for their teams

Join our community of data-driven dev leaders