PR Reviews – an Achilles heel of Agile Development

Everyone who worked on an Agile Team has been a witness to this behavior. Daily Standup is going great, everyone seems to make decent progress, until the very first development task is completed and the Story Owner creates a Pull Request (PR) and requests PR Review. This is the fine moment of truth, the one that measures the full extent of the team’s skill set in Agility and collaboration. 

If you’re lucky enough to still work in physical huddles and witness the body language of the engineers as “PR Review” is quietly mumbled, you will certainly relate to the emotional strain that can be observed at that moment. Every developer simultaneously has the same thought – “Oh, boy, PR Review. I don’t know… I’m too busy. I have to get my own work done.” Obviously this type of emotion is not the ideal growth mindset oriented environment Agile Teams are supposed to have and is something to work on, nevertheless this is a very frequent occurrence.

Then, someone finally says in a monotone “OK, I’ll do it” and things begin to look more optimistic, until the next day and the day after next, you continue observing the Story’s status remaining unchanged. The Story that took just a few hours to complete almost certainly gets stuck in Review Status for days on end. Developers continue to stay busy working on their own next set of stories, but at the Sprint’s end the Team finds itself with lots of incomplete work carried over to the next Sprint.

If this doesn’t sound familiar to you, then you haven’t worked on enough Agile Teams. Various studies have shown that most PR Reviews are completed in at least 1 day turnaround, however, that assumes that PR Review did not find anything that needed to change. If change to a PR was requested, a new PR Review will be necessary. And that, ladies and gentlemen, takes another day. Lastly, PR reviews take place before QA Review and its easy to see how this single step can grind the entire development cycle to a halt.

PR Review was introduced by Git to help create the discipline around code reviews and designed to help improve software quality and reduce the number of bugs. The intention is good, however is it worth it in practice? The challenge is that over time, as the number of Stories ready for PR Review increases, managing the daily priority of work for the team and keeping it moving to closure becomes really difficult. Adding insult to injury with remote teams spending very little face to face time, there are less expectations for immediate response from other team members and it’s easy to see how PR Reviews can become the biggest bottlenecks in getting things done.

There are really only two ways to solve this challenge. First, is to eliminate the PR Reviews entirely by changing to a more traditional code review that is purposefully scheduled and get an entire team into a meeting room to discuss. Another is to introduce a Policy for handling the PR Reviews correctly. Let’s take a look at each of these options.

The Case for Abolishing PR Reviews

Prior to PR Reviews being introduced by Git in 2008, developers typically had agreed on a set of coding standards and participated in Pair Programming, Pair Review, and Team Code Review sessions. The most beneficial format was a meeting with an overhead projector where one team member demonstrated to the team all of the coding changes that went into the implementation of given functionality. The beauty of this meeting was not just talk about implementation, but also review the design considerations, tests that were created, configurations used, and much much more. Think of it as a brown bag where someone had a chance to present their solution and all of the context and knowledge that went into the implementation.

The strength of the approach was how the team came together and to have open, interactive, insightful and synchronous conversation. PR Reviews changed this conversation from synchronous and interactive to asynchronous and reactive. Because folks could now comment and send each other messages, they naturally stopped getting together in meetings. The premise behind PR Reviews is that developers generally have the skill set and ability to review what is needed at their own time and if there are questions are sufficiently responsible to raise them via comments. And while it wasn’t a desire of PR Review to halt developers from coming together to collaborate, the byproduct of PR Reviews was that asynchronous “throw over the wall” behavior became pervasive. 

Not having deep enough context that was shared well in front of a wide audience in a traditional code review setting, the quality of PR Reviews generally dropped. Many times developers even go as far as to ask for favors in clicking instant approvals bypassing the entire review. Then, there are lots of reviews that focus too much attention on needless style related details and pointless preferences of the way software is written – which often presents itself as Technical Bullying that forces developers to iterate and make changes that drive absolutely no value to the end user. PR Reviews are very hard to do well – they aren’t a small task and require a significant amount of time and sufficient knowledge and context to do well.

Lastly, there are often teams where the culture within the team creates additional problems exacerbated by PR reviews. Imagine a scenario where a senior level team member assumes a very firm “code owner” position, essentially not allowing anyone to contribute without his absolute approval, becoming a bottleneck by being physically unable to cover all reviews in a reasonable amount of time. 

Before PR Reviews, the engineering culture was such that quality talent was hired so that they can be trusted to code well, and code reviews were not just about code quality, but also a sharing and learning opportunity. After all, isn’t this why companies try to hire the best talent – to trust them to do the job well? And if so, is PR Review even necessary?

PR Review Policy for avoiding Development Bottlenecks

PR Reviews are not entirely useless. One would argue that in the right environment, they are critical and necessary even with talented and highly skilled individuals. Afterall, everyone can make a mistake or miss something. There are other examples, such as a young team with lots of Junior or new Developers who may not have enough knowledge and experience in a given area, where PR Reviews can offer a last line of defense. Another, is high risk software in which a bug or coding mistake may have much bigger consequences than the effort to participate in the reviews themselves.

What’s clear is that there is a need to establish a good working Policy on how the team should handle PR Reviews. A modern growing trend is to promote a Policy of short circuiting the development process if PR Review has not been performed within a given timeframe by promoting the code to the next step automatically if the work is waiting for PR Review for too long. While popular, this is a mistake. Especially in high risk environments as mentioned above. Is there a better way?

The answer is yes. Borrowing from the discipline of Flow and the optimal management of Work In Progress items. The best PR Review Policy is when someone volunteers to conduct a PR Review, the developer is required to stop all work to complete PR Review before proceeding with the rest of the work. 

At first sight, this policy seems like it creates a lot of interruption for developers and forces them into context switching. However, that is merely an initial reaction. The reason why this approach is effective is that it minimizes the Work In Progress by allowing PR Reviews to occur sooner and allowing work to be completed. This in turn reduces development Cycle Times and refocuses the team on the remaining work, rather than constantly backtracking. 

Conclusion

PR Reviews are here to stay. And while abolishing them can have a lot of value for some teams, this may be hard to do. With remote work, PR Reviews are heavily relied on and with the right team and enough attention they can deliver a lot of value. So they aren’t likely to go away. However, it’s important to make sure that they work for your team and have the proper Policies defined around them in order to avoid becoming a bottleneck that creates more work and prevents timely delivery.