Contact
Menu
Contact

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.

facilitating-retrospectives

A Little Non-dogmatic Technical Mentorship
Can Go A Long Way

 

Do you ever feel like your team is running 1,000 miles per hour in the wrong direction, and you're not sure why? Sometimes, a chat with a peer is all it takes to give you a meaningful perspective to help you course correct, or to give you the courage to continue on your current path. 

For friends of Stride, we're offering a free Peer Chat. Tell us what's on your mind, and we'll share our experiences. Don't be shy, we've been there ourselves.

Subscribe by Email

Comments (1)