Daniel Irvine

The problematic pull request

Code reviews are a wonderful thing. Multiple pairs of eyes make for higher quality code. Reviews help spot errors and also serve as a teaching tool. You write a chunk of code, pass it on to someone else, and they comb through it for errors and possible improvements. They provide you with suggestions and you then go back and change the original code.

However, the processes by which we perform reviews can be deceptively ineffective. This is particularly true when we use tools that replace direct in-person communication with written online reviews, such as GitHub’s pull request system.

There are a couple of common issues with code reviews.

The first is that code reviews vary in effectiveness. The reviewer can be careful, careless, or somewhere in-between. Even the most meticulous of reviewers have days when they’re just too busy to devote enough time to the task. And sometimes the amount of code we’re asked to review is an issue: if there’s more than a few hundred lines of code then we’re likely to do a sub-par job of reviewing.

The second issue is that code reviews take up a great deal of time. In addition to the time spent reviewing code and correcting the code, there’s the hidden time cost of context switching. Both author and reviewer need to schedule time to do the extra pieces of work.

Code review tools

Tools like GitHub’s pull request system aim to solve these issues. They:

Together these features relieve you of the organizational work associated with code reviews. Commenting becomes a breeze.

However, there are now three additional problems:

  1. Delaying the merge stops code from being immediately useful. This is a good thing if the code contains errors, but if it doesn’t then the code becomes stale, and becomes more stale as time passes. If the rest of the development stream is changing in the meantime then extra work needs to be done to keep this code up to date.
  2. Written feedback is slower to deliver than in-person feedback. Coalescing thoughts into writing is slower than simply having a conversation. Two people sitting beside each other can correct code together. With code review tools, unclear comments require clarifying follow-up messages, so conversations stutter along in a stop-start fashion.
  3. The ease of commenting may engender a culture where each pull request ends with multiple nit-picky comments from multiple reviewers, overwhelming the original author with additional work.

All of these problems are quite serious. Submitting a code review becomes deeply demoralizing, unless you’ve got nerves of steel.

To counter these problems, authors begin batching up changes into larger, less-frequent reviews, which only makes the problem worse. Under these conditions, the average time from commit to merge can easily spiral out of control, and developers will be mentally exhausted.

Pull requests have their use

None of this is to say that pull requests are not useful. Open-source projects, for example, are reliant on tools to organize discussion between vast, weakly connected networks of developers. The success of these projects hinges on how easy and enjoyable the authoring experience is for contributors. So it’s critical that these projects do not fall into bad habits with their code review process. If there are any road blocks, then contributions will die off.

Of course, that’s a serious risk for all projects, but commercial projects generally do not get killed like that, they just trundle along until the money runs out.

Taming your pull requests

Here are a few ideas you can apply to streamline your pull requests.

As I said at the beginning, code reviews are a wonderful thing. They can be very worthwhile. But remember to frequently retrospect on your processes—on all of them, not just your review process. Critical thinking is always required. So ask yourself: are your tools helping you or are they hindering you?

Originally published on 8th Light.