Those “Pesky” Pull Requests are Totally Worth It

Pretty much everyone does code reviews. They’ve been around a long time. I remember back in my Borland days when the Chief Scientist would come in every morning and review all the code that had been checked into the Subversion(!) repository the previous day and send emails out to folks whose code wasn’t up to snuff. That’s old school.

Slightly less old school? Saving all the check-ins up until Friday for the Dev Leads and/or Dev Managers to review and approve. Both of these techniques leave a lot to be desired -- the main thing being a complete lack of interaction between the developer, the code, and the reviewer. 

Code Reviews have a number of purposes. Probably the most important one is preserving the quality and integrity of the code in the repository. Even the two old-school ways above do that. 

But almost as important is the learning opportunity that code reviews can provide. If the only feedback a developer gets from a code review is mistakes in formatting or other trivial things like that, then nobody learns and gets better. The old school ways above provide for few opportunities for a developer to increase their skills.

To provide learning opportunities, code reviews evolved into meetings where everyone looked at the code written that week and commented on it, criticized it, or otherwise ran it through the gauntlet. This did provide a learning opportunity for developers, but it took more time, as it was 100% synchronous and required all code to wait for the next scheduled meeting to be reviewed.

Now, almost no one is doing these old-school code reviews anymore. All the cool kids are doing pull requests. (Some folks call them “merge requests.”)  Pull requests have a number of advantages over the previously mentioned methods, including:

• Being done completely asynchronously, but in public for all to see.
• No one needs to wait to review the code -- it can happen almost immediately after a pull request is issued. 
• A history of all the comments stays with the code in a repository. This allows a developer to come back to the code a year later and see all the thought that went into writing it.
• Pull Requests can be tracked, monitored, and measured. A whole lot of good things can come out of that. 

Should you do code reviews at all?

Interestingly, some say no, you shouldn’t.

Not only is Jessica Kerr a great speaker and a good Twitter follow, but she also has some interesting ideas about code reviews in her article of March 27 entitled "Those pesky pull request reviews." In fact, she doesn’t like pull requests, and argues that you should sidestep them by just working on a given task as a team, so that everyone sees everything as the work gets done. 

She believes that pull requests work great for open source projects where a “team” is really a set of individuals coordinating work together. For true development teams, she believes that if a team all works together on a single task, everyone learns and understands the code, and thus there is no task switching between coding and doing pull requests because the pull requests are unnecessary. 

Jessica’s idea is radical -- basically going beyond Pair Programming and moving into mob programming. Mob programming is the idea of having whole teams work together on projects in serial rather than individually in parallel. Mob programming can eliminate the need for pull requests by causing all of the communication and learning to take place during the coding phase, without any review. 

Not a Fan

I’m having a hard time agreeing with her idea for a couple of reasons:

1. The transaction costs are too high. It seems to me that having four people work on a project together makes for many communication channels, increases the likelihood of interruptions, and reduces the amount of code that will actually get written. It’s sort of a “Too many cooks spoil the broth” notion. 
2. It doesn’t capture the discussions and history that will remain long after the code is committed. One of the most important and powerful benefits of pull requests is the learning that can take place during and even long after code has been reviewed and deployed. 
3. Finally -- not all projects are conducive to multiple team members working together. Some are small and multiple people working together would be overkill. Some are esoteric and require the focus of one person. Some will match the team and can be worked on together. There’s no one size fits all solution for all projects.
4. Finally, not doing pull requests pretty much eliminates all the benefits of metrics systems like LinearB. Tracking the progress of pull requests and code reviews through the pipeline is a critical process for knowing how your team is performing. Without that, you can measure things and if you can’t measure things, you can’t improve.  

As part of a discussion about code reviews, Rob Kraft, one of the Development Leaders in our vibrant Dev Interrupted Discord Server (you should join!) made the following comment that I agree with:

I think that what Jessica needs is a good look at LinearB.  🙂

Let me address some of her more specific objections:

1. “Let’s face it: nobody wants to review pull requests.” Well, I don’t think that is true. We here at LinearB see customers every day that are doing pull requests efficiently and effectively. Sure, pull requests can be hard and nobody wants to do them if you aren’t correctly incenting the team to create pull requests that are easy to review. No one likes a huge pull request. But through monitoring metrics like Pull Request Size, you can encourage your team to create small, easy-to-review pull requests. And voila! People don’t hate pull requests anymore.
2. “They’re a social interaction minefield!” People complain that code reviews can cause strife on a team. Well, so can conversations during Mob Programming. I’m not sure that I see a distinction, And if doing a code review causes strife, then you have a cultural problem that no development methodology is going to solve. 
3. “We could blame the people. We could nag them more. We could even automate the nagging!” Well, if code reviews are small, concise, and easy to do, “automating the nagging” via our WorkerB product is usually more than enough to get the ball rolling and keep it rolling. Notifications and tracking of any reviews that do happen to languish keep things moving as well. LinearB customers have seen drastic improvements in code pipeline productivity as a result of this so-called “nagging”.
4. “Maybe instead of trying to work a bit more together, we could work together.” Well sure, but if you do that, checking in code without a process of pull requests and code reviews, well, then you aren’t getting all the benefits listed above, nor those of a metrics tool that can show you what your Cycle Time is doing. And I don’t believe that mob programming will prevent the cultural problems that can arise from code reviews. People will be people whether in a mob programming environment or in an asynchronous code review process.

Bottom Line

Okay -- so what rubber is hitting the road here?

If pull requests and code reviews are hard and people don’t want to do them, then you are doing them wrong. So the trick is to make them easy to do.

We here at LinearB see many, many customers improve their Cycle Time and their overall software development process by using and tracking pull requests. By combining metrics tracking around pull requests with tools like WorkerB, many, many development organizations have seen smaller pull requests, better reviews, shorter Cycle Times, and an overall sense that things are really humming. 

Monitoring things like the size of pull requests, when pull requests are assigned, picked up, and commented on, as well as monitoring the depth of reviews that take place all create an environment of small, discrete, easy to review pull requests. 

And of course, if you want to find out more about what our customers already know, you can book a free demo of LinearB.

In the end, while her ideas are intriguing and thought-provoking,  I can’t say I agree with Jessica’s argument. There doesn’t seem to be any good reason not to do pull requests with code reviews.

Jessica’s blog post can be read on her Jessitron blog and you can follow her on Twitter at @jessitron

Join the Dev Interrupted Community

With over 2500 members, the Dev Interrupted Discord Community is the best place for Engineering Leaders to engage in daily conversation. No sales people allowed. Join the community >>