What do you look out for in your code reviews? Is the quality of your code reviews consistent? Do you use a code review checklist?
There are several reasons why the quality and thoroughness of code reviews may vary significantly across pull requests. Some reasons for inconsistency in code reviews include:
The skill and competence level of the code reviewer.
How much the code reviewer knows about the specific issue(s) addressed in the code change.
The time of day when the code review was performed.
The specific programming issues that have been on the reviewer's mind lately.
The reviewer's perception of the code contributor's competence.
The programming languages and technologies used in the code change.
A code review checklist is a useful way to improve the quality and consistency of code reviews. Consistent use of checklists decreases the likelihood that a reviewer's focus is skewed toward specific areas of interest or convenience at the expense of other important areas.
Dr. Michaela Greiler has done some remarkable work on helping software engineers improve their code reviews. This post presents a checklist, based on Dr. Greiler's work, with 62 things to look out for in code reviews. The questions in this checklist are structured such that each question can be answered with either a YES, NO, or Not Applicable (N/A). In theory, the likelihood that a code change is of acceptable quality is inversely proportional to the number of “NO” answers to the questions in the checklist. Many of the questions in this checklist are subjective and almost completely at the discretion of the code reviewer.
Does this code change use existing code whenever possible instead of introducing new code?
If this code change does not use similar existing functionality in the codebase, is there a good reason why?
Does this code change use best practices, design patterns or language-specific patterns in ways that improve code maintainability, readability, performance, and security?
Does this code change use data in a way that minimizes or eliminates privacy concerns?
If this code change 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 code change adequately avoid exploitation of behavioral patterns or human weaknesses?
Does this code change adequately ensure that it does not cause mental or physical harm to users?
Does this code change adequately ensure that it does not exclude certain groups of people or users?
Does this code change adequately avoid introducing any algorithm, AI or machine learning bias?
Does this code change adequately avoid introducing any gender/racial/political/religious/ableist bias?
Testing and Testability
Is the code well designed from a testability perspective?
Does the code change have enough automated unit, integration, and system tests? If not, do the existing tests adequately cover the code change?
Do the test cases in the code change cover all important inputs or edge cases you can think of?
Is sensitive information like user data and credit card information securely handled and stored?
Does this code change use appropriate encryption schemes when necessary?
Does this code change properly hide secret information like keys, usernames, and passwords?
Does this code change properly address security vulnerabilities such as cross-site scripting and SQL injection? Does this code change properly sanitize and validate user input?
Does this code change properly validate data retrieved from external APIs or libraries?
Performance, Reliability, and Scalability
Is this code change unlikely to have a significant negative impact on system performance?
Does this code change include all the useful performance optimizations that you can think of?
Does this code change have instrumentation for metrics reporting and failure alerts?
Does this code change use appropriate built-in functions wherever possible?
Has all debugging code been removed from this code change?
Are all performance optimizations in this code change necessary?
Does this code change use some form of caching whenever appropriate?
Readability
Is the code easy to understand?
Are methods and functions small enough to be easily readable?
Are function, method, and variable names meaningful?
Are modules located in appropriate and meaningfully named files, folders, and packages?
Is control flow in modules, functions, and methods intuitive?
Is the data flow in modules, functions, and methods intuitive?
Are all comments in this code change necessary and meaningful?
Do the comments in this code change describe why and not what?
Does this project use a linter?
Secondary Review
If this code change impacts different teams, has it been reviewed by people from the affected teams?
This code review checklist is also available as a spreadsheet to edit and improve as you please.
71% of Fortune 500 companies can't be
wrong – mentorship is crucial to career growth. Our free 'state of mentorship' shows
you the facts, stats and studies on this career superpower.
Including 10% discount on
your next session!
Your subscription could not be saved. Please try again.
Join 23,932 members receiving exclusive expert career tips
twice per month.
Change your career, grow into leadership, start a business, get a raise. Boosting
your
career is easier
with some help!
Join our value-packed newsletter to learn from the giants of the industry.