Myth vs. Fact: why is code review so hard? • Minimum Viable Blog
Minimum Viable Blog
Software construction, opinion, and self
A lot of teams procrastinate code review, phone it in, or get stuck in endless back-and-forth on every PR. Let’s talk about why.
Myth #1
Myth: Code review is difficult because it’s boring and pull requests are too long.
Fact: Code review is difficult because it duplicates the cognitive work of programming without all the intermediate steps.
Engineers tend to dislike reviewing code. Instead of just accepting it as an unpleasant part of the job, we should ask why.
Let’s start with the most common misconception: it’s not about PR length. Some teams have discovered that establishing a norm of smaller PRs leads to faster review times and less mental fatigue, and I wholeheartedly support this, but it’s not the PR length alone that brings these benefits. It’s the fact that when engineers are encouraged to submit shorter PRs, they compensate by doing more work to organize and document each PR.
Suppose you need to make a code change that, in its ideal form, comes in around 1000 lines. If your team has established a norm of PRs being 200 SLOC or less, you’ll need at least five PRs, and if you want those PRs to make sense as individual units, you’ll have to do a lot more storytelling:
This PR updates the database with new columns; here’s what they’re for.
This PR updates DTO classes so the data in those columns can be sent over the wire.
This PR refactors a service to incorporate the new data into its calculations.
Etc.
Small PRs are easier to review because they make code authors explain themselves better—step by step, at a finer grain. Without that explanation, the code reviewer has to try to to reconstruct all the reasoning and context that went into the PR. This abstract guessing game can easily take more mental energy than it took to actually write the code. We think of code review as a quick checkbox task, but the way many teams approach it, it’s the most mentally taxing thing they do.
When you code, you spend most of your time researching. You clarify requirements, explore the relevant parts of the codebase, read documentation, try out different approaches, and finally land on a solution you like. When you’re finished, the code you submit is a two-dimensional snapshot of N-dimensional context. It’s lossy compression. The snapshot can be projected back into a model, but unless information is provided by other means, there will be significant gaps.
There are several ways to narrow those gaps:
Split up large PRs into smaller ones (perhaps targeting a feature branch).
Document and share ahead of time what problem you’re solving, how you’re solving it, and why you chose that approach.
Leave comments throughout your own PRs, explaining your thought process around less obvious changes.
Use pair programming or mob programming to share context as you build.
Use interactive rebasing (or the equivalent in your source control tool of choice) to rearrange commits and make PRs reviewable in small chunks.
We can make code review easier for each other, and we should. Shorter PRs are just one way to accomplish that.
Myth #2
Myth: The primary purpose of code review is to block bad code from getting into the application.
Fact: The primary purpose of code review is to align the team.
Don’t get me wrong, it’s important to look out for each other’s mistakes. Security vulnerabilities and critical bugs are often caught in code review, and they should be, though it’s better to catch them earlier—more on that in a moment.
Code review’s primary purpose, though, the purpose that consistently brings the most value, is alignment. Code review is a mechanism by which a team affirms:
We understand what changes are being made.
We understand why these changes are being made.
We can commit to maintaining this code.
Note that there’s nothing here about agreement. Agreement and alignment are different things. Team members may disagree about how to solve a problem, but if they share an understanding of the problem, along with the pros and cons of different approaches, they can commit to an imperfect solution and move forward anyway. That’s alignment.
A secondary purpose of code review is mentorship. Team members can use it to teach techniques, share ideas, and deepen each other’s knowledge of both the immediate context of the application and broader technologies (e.g. programming languages). One of the best kinds of code review comments is, “I think we could shave off several lines of code by using this technique/feature/class: [link].” Code review shouldn’t be the only place this happens, or even the main place, but it shouldn’t be discounted either—the things you learn from PR comments tend to stick with you, since by default...