A good code review can feel like magic. A skilled code reviewer will deftly work through each part of a pull request, noting potential problems and proposing elegant solutions. If you’re like many developers, you recognize a good code review when you see one. You’ve probably even created a few yourself. But you’re probably also a bit unsure as to how to make sure that your code reviews are good every time.
One way to improve your code reviews consistently is to create a code review checklist that you run through every time you review code. A checklist makes sure that you don’t forget anything. At the same time, that checklist will stop you from turning the code review into a giant slog. You don’t want a simple code review to take hours, but you also don’t want to be the person who approves sketchy code, either.
In this article, we’ll break down the core parts of a pragmatic, lightweight code review checklist that you can use with your team.
Why Code Reviews Are Important
Before we start, it’s important to go over why code reviews are important. Many teams don’t do code reviews, or they simply use them as a rubber stamp to approve new code. Neither of those types of teams gets any value from code review.
Code review should be used as an opportunity to improve the code quality for your team. A team which performs good code review will find that over time, they write fewer bugs and develop more features. The trade off for these benefits is that developers need to spend time reviewing code and not writing it. On teams that perform good code reviews, this is a good trade off. The key is to make sure that your team is a team that performs good code reviews.
Creating a Good Code Review Checklist
It’s important to remember, before we dive in, that this list isn’t exhaustive. It’s intended to be language-agnostic; if you’re writing a language like C++ you’ll probably want to add a code review checklist item to make sure that there aren’t any pointer or null reference issues. This is simply a good start that you can use to build on as you gain experience.
Does the Code Do What It’s Supposed to Do?
This is, without a doubt, the most important part of any code review. As a reviewer, you should be able to say that code does what it’s supposed to before you approve it to be merged. This first step can also take more time than all the rest of the steps. For some code reviews, you’ll find that it’s easy to tell whether the code does what it’s supposed to. For instance, a styling change in a single UI component is probably pretty easy to verify. A change to a data processing pipeline that handles a dozen different data types? Probably a bit more complicated.
Sometimes, checking off this part of the code review might mean pulling the code to your local machine and testing it out manually. I’ve worked with teams that require every pull request to include instructions from the author explaining how developers should test the functionality of the code. I find this to be a great way to make sure that everyone can confirm new code does what it’s supposed to.
Is the Code Easy to Understand?
After correctness, legibility is the next most important attribute for code. While code is written to be consumed by computers, it’s written to be used by people. Unreadable code is much harder to modify, and drastically increases the growth of technical debt within the code base. Detecting code that’s difficult to understand is actually pretty simple. If you need to re-read a section of code two or three times to understand what it does, it’s probably difficult to understand. That doesn’t automatically mean the code needs to change. Some types of code will necessarily be difficult to understand (concurrency code is an excellent example of this).
While there is no universal standard for “easy to understand” code, there are some basic guidelines. By ensuring code is readable, you save time for everyone whoever has to maintain it. Often, that’s going to be you, in the future. Be kind to your future self!
Is the Code Well-Documented?
While it’s important to provide code that’s easy to read, code documentation is also important for your code base. While easily-understood code is easier to maintain in the future, well-documented code is easier to use today. Good code documentation should clearly explain what functional units of the code base expect (their inputs), how they work (their function) and what they return (their outputs).
Many teams will save code documentation for immediately before the release of a project, but this is a bad idea. Code is most fresh in our minds immediately after writing it. If code documentation is part of a code review, the information that documentation contains will be more useful to the people who utilize that code, for whatever reason.
Does the Code Pass Automated Style Guidelines?
Another way to make code easier to understand is to implement team-wide code style guidelines. This is a great idea. However, you don’t want to check code’s adherence to these guidelines by hand. The reality is that there are lots of terrific automated style checkers for just about any coding language. The more advanced of these tools can integrate right into your continuous integration pipelines. This means that you’ll know right on the code review itself whether or not the code matches the team’s style.
By writing code that adheres to a specific style, developers are more easily able to work in varying parts of the code base. This flexibility improves the overall versatility of the team, and allows each member to grow by working on a variety of tasks.
Is There Code Duplication?
There exist automated tools that can find copied and pasted code, but this review item is a bit more subtle than that. A good code reviewer will identify pieces of code that functionally do very similar things. They’ll then suggest ways to refactor the code to synchronize those similar bits of code.
Code bases with limited repetition are easier to refactor and enjoy quicker bug fixes.
?Does the Code Follow Defined Architecture?
Mature software teams have defined architectural guidelines for applications to follow. A good code reviewer knows and understands those guidelines, and can see when code is slipping outside defined boundaries. When reading through code, ask yourself whether there already exists a tool in your code base that performs functions similar to what this bit of code does. If there is, you should recommend refactoring the code to use those common areas, instead.
By focusing your team toward existing architecture for new features, you reduce the likelihood of shipping new bugs. You’ll also keep your code easy to understand.
Keep Your Code Review Checklist Simple
A common mistake I see from novice code reviewers is that they think it really is magic. They see more experienced reviewers providing in-depth feedback, and feel like they need to do the same. The truth is that it’s just as important for code to be readable, concise and working correctly as it is for it to be well-optimized or written in comparatively few lines. Instead of trying to fix every problem your team has in a code review, spend time making sure that just this little pull request is good. You won’t fix your code base over night, but you will find it’ll get better over time.
The same will be true for your own reviewing skills. You won’t become a perfect code reviewer in a day or two. There aren’t eight easy steps. Instead, just like with coding, the key to getting better is to practice. By following this checklist, you’ll be practicing the right way, every time. Before long, you’ll probably find that you don’t need it at all.
Learn more how CodeIt.Right can help you automate code reviews and improve the quality of your code.
1 Comment. Leave new
[…] Building a Pragmatic, Lightweight Code Review Checklist – Eric Boersma […]