Code review is one of those hygienic things most developers believe they ought to do, like kitchen workers washing their hands, but don’t always do. Yet tech articles tend to be unanimously in favor of code review, extolling its benefits (“catch bugs”, “maintain code consistency”, “transfer knowledge”). So if restaurants can require workers to wash their hands, maybe organizations can require their developers to review every code change?
Broadly speaking I think we can lump all of code review’s benefits under the category of “improve code quality”. I know there is no agreed-upon definition of code quality but allow me to channel my inner Justice Potter and wave away your objections for now. (What of “transfer knowledge”? - first I think unfamiliar reviewers reviewing code changes is inefficient compared to them interacting with the code by making a small change and/or writing tests for it. Second whatever knowledge is accumulated by the reviewer should ultimately lead to higher quality code output anyway).
Context
Do you think every code change to a life-support system should be reviewed? What about code that just controls adverts? Does your assessment change if the advertising code is being burned directly onto silicon chips which control billboards?
Code review happens in a context. It is more valuable in high risk/severity contexts than otherwise. If you require your developers to review every code change, the stakes should be high, else they will know it’s bullshit and the process will become a “LGTM” parade. Mandating code review might make it more likely for bugs to sneak through, especially when there is a deluge of changes to be reviewed. A 2018 study of Google’s mandatory code review practices included this nugget:
The majority of changes are small, have one reviewer and no comments other than the authorization to commit
The study also reported that 70% of changes are approved within 24 hours. Now, a favorable interpretation would be that the code change submissions are of such high quality that they are quickly approved without comment, but I think understanding human nature suggests otherwise.
Returning to knowledge transfer for a moment, a study of over 32,000 code-reviewed changes on GitHub found no evidence that the submitters' code quality increased in subsequent code changes. This suggests to me that the open source volunteer context degrades submission quality as the consequences are low compared to being an employee, but also that knowledge transfer is a complex process and perhaps not strongly associated with code review alone, but a supportive learning environment.
Change Risk
Even if the organizational and system contexts are the same, not all code changes are equally risky. You can probably guess the important factors:
- Submitter familiarity with the codebase and general level of experience
- Number of files/lines of code changed
- Code complexity
- Does the code have a clear, single owner who is responsible for its performance?
- Does the change have tests, does the codebase have good test code coverage
Organizations can use this knowledge to use code review when it’s most valuable. It could be as simple as only reviewing code submissions from newbies (some Open Source projects do this), to as complex as building a system which flags higher risk changes for review. Obviously such a system based on hard metrics would be imperfect and flag too few/many changes for review. That would still be better than blanket mandatory code review and the system’s tolerance could be tuned to the needs of the organization. Using AI to predict bugs in code changes is an active research area which will yield more intelligent vetting behavior (imagine a change containing hundreds of the same one word substitution; it probably doesn’t need a review).
Despite the limitations of our understanding of code quality, keeping in mind the context and nature of a code change can help us decide when the opportunity cost of code review is worthwhile.