A code review checklist, as well as clear rules and guidelines around code reviews, are crucial. A code review checklist can make your code review practice so much more beneficial to your team and significantly speed-up code reviews.
Studies have shown that code reviewers who use checklists outperform code reviewers who don’t. So, consider using a code review checklist, whether you are a new developer or already an experienced one.
Code Review Checklist Overview
- Logic Errors and Bugs
- Error Handling and Logging
- Usability and Accessibility
- Ethics and Morality
- Testing and Testability
- Security and Data Privacy
Code Review Checklist for Code Authors
Code review checklists are not only something for the code reviewers. Instead, as the author of the code change, follow the code review best practice and be your own reviewer!
So, before sending out the code for review, make sure that:
- The code compiles and passes static analysis without warnings
- The code passes all tests (unit, integration, and system tests)
- You have double-checked for spelling mistakes
- You did a cleanup (comments, todos, etc.)
- You described what this change is about, and the reason for the change
Apart from that, you, as the code author, should run through the same code review checklist as the reviewer.
Code Review Checklist for Code Reviewer
As a code reviewer, it is your task to look for the most important issues first. It is easier to get hung up on nitpicking. But that’s not good.
In one of our large studies at Microsoft, we investigated what great code review feedback looks like. We clearly saw that comments revealing larger structural or logical problems are perceived as much more valuable than comments that focus on minor issues.
This is where code review checklists come into play. A great checklist directs your attention to the important and most valuable issues. Below you will find the checklist that I also use during my code review workshops. It is divided into ten sections. Each section guides you through several questions. So, let’s start:
- Does this code change do what it is supposed to do?
- Can the solution be simplified?
- Does this change add unwanted compile-time or run-time dependencies?
- Is a framework, API, library, or service used that should not be used?
- Could an additional framework, API, library, or service improve the solution?
- Is the code at the right abstraction level?
- Is the code modular enough?
- Can a better solution be found in terms of maintainability, readability, performance, or security?
- Does similar functionality already exist in the codebase? If yes, why isn’t it reused?
- Are there any best practices, design patterns, or language-specific patterns that could substantially improve this code?
- Does this code adhere to Object-Oriented Analysis and Design Principles, like the Single Responsibility Principle, Open-close principle, Liskov Substitution Principle, Interface Segregation, or Dependency Injection?
Logic Errors and Bugs
- Can you think of any use case in which the code does not behave as intended?
- Can you think of any inputs or external events that could break the code?
Error Handling and Logging
- Is error handling done the correct way?
- Should any logging or debugging information be added or removed?
- Are error messages user-friendly?
- Are there enough log events, and are they written in a way that allows for easy debugging?
Usability and Accessibility
- Is the proposed solution well-designed from a usability perspective?
- Is the API well documented?
- Is the proposed solution (UI) accessible?
- Is the API/UI intuitive to use?
Ethics and Morality
- Does this change make use of user data in a way that might raise privacy concerns?
- Does the change exploit behavioral patterns or human weaknesses?
- Might the code, or what it enables, lead to mental and physical harm for (some) users?
- If the code adds or alters ways in which people interact with each other, are appropriate measures in place to prevent/limit/report harassment or abuse?
- Does this change lead to an exclusion of a certain group of people or users?
- Does this code change introduce any algorithm, AI, or machine learning bias?
- Does this code change introduce any gender/racial/political/religious/ableist bias?
Testing and Testability
- Is the code testable?
- Have automated tests been added, or have related ones been updated to cover the change in functionality?
- Do the existing tests reasonably cover the code change (unit/integration/system tests)?
- Are there some test cases, input, or edge cases that should be tested in addition?
- Were updates to documentation, configuration, or readme files made as required by this change?
- Are there any potential impacts on other parts of the system or backward compatibility?
Security and Data Privacy
- Does the code introduce any security vulnerabilities?
- Are authorization and authentication handled correctly?
- Is (user) input validated, sanitized, and escaped to prevent security attacks such as cross-site scripting or SQL injection?
- Is sensitive data like user data or credit card information securely handled and stored?
- Does this code change reveal any secret information like keys, passwords, or usernames?
- Is data retrieved from external APIs or libraries checked for security issues?
- Do you think this code change will impact system performance in a negative way?
- Do you see any potential to improve the performance of the code significantly?
- Is the code easy to understand?
- Which parts were confusing to you and why?
- Can the readability of the code be improved by smaller methods?
- Can the readability of the code be improved by different function, method, or variable names?
- Is the code located in the right file/folder/package?
- Do you think certain methods should be restructured to have a more intuitive control flow?
- Is the data flow understandable?
- Are there redundant or outdated comments?
- Could some comments convey the message better?
- Would more comments make the code more understandable?
- Could some comments be removed by making the code itself more readable?
- Is there any commented-out code?
Next to the 10 topics on this code review checklist that you should focus on, you can also think about whether someone else should have a look at this code.
- Do you think a specific expert, like a security expert or a usability expert, should look over the code before it can be accepted?
- Will this code change impact different teams, and should they review the change as well?
Well, that’s it. You looked and thought about the most pressing issues with this code review checklist. Congratulations!
Now, one of the exercises that I do in my code review workshops is to reflect with the participants on this checklist for code reviews by answering three questions:
- Which parts of the code review checklist are you focusing on the most?
- Which parts of the code review checklist do you tend to neglect?
- Do you believe some of those points are more important for a checklist for code inspection? Why?
But what about coding styles and conventions?
Maybe during this exercise, you realized that I did not check whether the code follows the right coding style. So, is that not important?
Short answer: it is important. Crystal-clear coding style guides are the only way to enforce consistency in a codebase. And consistency makes code reviews faster, allows people to change projects easily, and keeps your codebase readable and maintainable.
Google is a great example of doing this right. And keeping consistency with naming, best practices, and style throughout the codebase surely allows Google to have one of the fasted code review turnaround times.
As a starting point, I recommend using the ready-made coding styles for many languages from Google.
It is important to set the ground rules, but make sure to do that once and for all. Don’t argue about it on an ongoing basis. Check out this article that outlines how to get a team to agree on a coding standard.Cristal-clear coding styles can speed up your code reviews. But only if you automatically enforce them via tooling. Click To Tweet
Automate what can be automated
But, once you decide how your codebase should look, take the time to install and configure tooling properly so that code formatting becomes a matter of pressing a button.
Also, there is much more you can do. Use static analysis tools to free up the time of your human code reviewers. It is worth the initial effort.Don't make your reviewers check for issues tooling could detect more reliably and cost-effective. Click To Tweet
Be respectful and kind
Finally, the quality of the code review feedback does not only depend on WHAT you are saying but also on HOW you are saying it. So, the best code review feedback is worth nothing when it isn’t carefully phrased, humble, and kind.
For starters, phrase your feedback as suggestions instead of demands. For example, instead of writing “Variable name should be removeObject.” say, “What about calling the variable removeObject?”. For more input, read my article on how to give respectful code review feedback.
There is more for you…
As you should have this code review checklist always next to you, I prepared a beautiful printable version for you. You can download the code review checklist as a printable PDF for free here. Alternatively, check it out on GitHub, and don’t forget to star it. Another resource that might be super valuable for you is my code review e-book.
Finally, did you know I help teams make code reviews their superpower? Have a look at my remote code review workshop.
This article first appeared on michaelagreiler.com