The Unnoticed Costs of Code Review by Pull Request

Feb 03, 2017

Pair programming is probably the most controversial and least adopted agile practice. Capers Jones finds that for defect removal it’s relatively expensive compared to other techniques, though he doesn’t take other advantages of pair programming into account. (In his comparison of methodologies, XP as a whole comes out at or near the top in terms of schedule and total cost of ownership.) There is no denying that pairing is a highly visible cost to management: “Two developers at one workstation? No way! We already pay them a fortune!”

Start Increasing Productivity & Collaboration in Your Agile Team

Learn More

Research shows code review to have a huge impact on code quality. Pair programming ensures not only that all code is reviewed, but also that it’s reviewed as it is being written. A popular alternative involves using pull requests together with distributed version control systems such as Git. A single developer writes code on a branch, and when the code is ready to merge to the main line of development, they request a review. Automated tooling can keep code from being merged before being reviewed by a certain number of people. The tooling normally lets reviewers see a comparison view of the code before and after the proposed changes, with the differences highlighted.

This process lets developers review code individually, each at a convenient time. The cost of review is not as apparent to management and is therefore more likely to be accepted as part of the development process.

However, there are several hidden costs to this approach that could impact the project.

Siloed teams

The pull-request process was designed for open-source projects, where contributors are isolated, work separately, and may be anonymous. They may not know the code well, even dropping in for just a single commit. The technique lends itself to having a developer work alone on a separate feature, rather than the whole team delivering a single story collaboratively. Thus the tendency is for developers to specialize in their own part of the codebase, increasing the costs of coordination and the risks involved in losing a developer.

Weakened reviews

This isolation weakens reviews. As Theodore Nguyen-Cao writes, “Everyone seems to be involved in the process, but there is no vested interest. They are just looking at some code and asking themselves, ‘Does this code look good, and is it correct?’ It’s a very passive standpoint.”

Delayed feedback

By the time a review happens, the code is already written. In general, the more time passes between the introduction of an issue and its detection, the costlier it is to fix.

Tolerance of gold-plating

Sometimes, extra time spent polishing code is worth the cost; sometimes it isn’t. Once it’s polished, however, there’s no point in spending even more time unpolishing it—unless the polishing adds complexity that increases future maintenance costs.

Context switching

Developers have to focus not only on working on a feature, but also on the reviews they have to do. Incoming pull requests are a constant source of interruption. Once a feature is reviewed, a developer may need to return to it later to make changes. The resulting context switching adds costs, and the developer also experiences the distraction of many unfinished tasks.

Increased cycle time

It can be a long time before all reviews are finished, particularly if reviewers are busy. Review delay can often make it impossible to complete a feature within a single iteration. In addition, a developer waiting for a feature to be merged often goes on to work on a second feature instead of simply waiting. The result is that work in progress increases. If the first feature requires changes after review, these changes often need to be integrated separately into the second feature’s code, as well.

Using pull requests for code review was designed for open-source projects with isolated, sporadic contributors; it may not be appropriate for a collaborative team all continuously working on the same project. Before choosing this route, it’s worth considering the costs and how you might mitigate them.

Want to learn cost-effective methods to boost your team’s productivity and quality of work? For agile teams, proper use of retrospectives is key to a consistently improving team. Our free guide covers everything you need to know to facilitate your retrospectives like a pro.


You May Also Like

These Stories On Process

It's the third week in a row that your team has given the same update: "We are almost done with the 'previous orders' feature." You are frustrated with the lack of progress and the inability to shift to more important work—like that feature you wish the ... read more

Josh Seiden, the fourth speaker in Stride's Leadership through adversity speaker series, spoke about the value of focusing on outcomes, defined as: "measurable changes in behavior that drive business results."   read more

To start his talk, psychiatrist and psychoanalyst Dr. Kerry Sulkowicz admitted to being at a bit of a loss when asked about best practices for leading through a pandemic. "The honest answer is I don't know," he stated, "because none of us has lived ... read more

Taking direct aim at the narrative of the "all in" entrepreneur who takes extreme risks and depletes their bank account before ultimately succeeding, noted NYC VC Charlie O'Donnell started his Stride Consulting “Leading through Adversity” talk on May ... read more

Get Email Updates

Comments (1)