A code review checklist, as well as clear rules and guidelines around code reviews, can make your code review practice so much more beneficial to your team and significantly speed-up code review times.
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 for the code submitter
As the author of the code change, you should 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 and that you did a cleanup (comments, todos, etc.)
- You outlined what this change is about including the reason for the change and what changed
Apart from that you, as the code author, should run through the same code review checklist as the reviewer. Follow the code review best practice: Be your own reviewer first!
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 in nitpicking but comments revealing larger structural or logical problems with the code are much more valuable.
Have a look here to understand what makes for great code review feedback. And remember, the best code review comments aren’t demands, they are collegial suggestions. So instead of writing “Variable name should be removeObject.” say “What about calling the variable removeObject?”.
But enough of the sidetracking here comes your code review checklist.
- Does this code change do what it is supposed to do?
- Can this solution be simplified?
- Does this change add unwanted compile-time or run-time dependencies?
- Was a framework, API, library, service used that should not be used?
- Was a framework, API, library, service not used that could improve the solution?
- Is the code at the right abstraction level?
- Is the code modular enough?
- Would you have solved the problem in a different way that is substantially better in terms of the code’s maintainability, readability, performance, security?
- Does similar functionality already exist in the codebase? If so, why isn’t this functionality reused?
- Are there any best practices, design patterns or language-specific patterns that could substantially improve this code?
- Does this code follow Object-Oriented Analysis and Design Principles, like the Single Responsibility Principle, Open-close principle, Liskov Substitution Principle, Interface Segregation, 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?
Testing and Testability
- Is the code testable?
- Does it have enough automated tests (unit/integration/system tests)?
- Do the existing tests reasonably cover the code change?
- Are there some test cases, input or edge cases that should be tested in addition?
- If this change requires updates outside of the code, like updating the documentation, configuration, readme files, was this done?
- Might this change have any ramifications for other parts of the system, backward compatibility?
Security and Data Privacy
- Does this code open the software for security vulnerabilities?
- Are authorization and authentication handled in the right way?
- Is sensitive data like user data, credit card information securely handled and stored? Is the right encryption used?
- Does this code change reveal some secret information (like keys, usernames, etc.)?
- If code deals with user input, does it address security vulnerabilities such as cross-site scripting, SQL injection, does it do input sanitization and validation?
- Is data retrieved from external APIs or libraries checked accordingly?
- 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?
- Was 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 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?
- Do you think a specific expert, like a security expert or a usability expert, should look over the code before it can be committed?
- Will this code change impact different teams? Should they have a say on the change as well?
Well, that’s it. You looked and thought about the most pressing issues. Congratulations!
But what about coding styles and conventions?
Yes, it is important that your team or company has crystal-clear coding style guides. Because this is 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 this 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.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
Once you decided how your codebase should look like, take the time to install and configure tooling properly so that formatting the code correctly becomes a matter of pressing a button.
But 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 reliable and much more cost-effective. Click To Tweet
There is more for you…
If you want to get the most out of your code review process, make sure to also check out my ultimate code review blog post series showing you code review best practices, code review pitfalls, as well as how to give great code review feedback. You can also find case studies on how code reviews work at Microsoft or how Googlers review code.