I recently read Why your team doesn't need to use pull requests. The author, Kief Morris, comes from ThoughtWorks and wrote Infrastructure as Code so I was excited to review this with my team. (Sam Newman made similar arguments as part of his 2017 conference talk Feature Branches and Toggles in a Post-GitHub World.)
Here’s my high level summary of their ideas:
You don't need pull requests because you have CI, and Pull Requests slow down the merge process.
I agree pull requests slow down the merge process. But I’ve yet to find anything more effective as a general purpose tool to improve your team through knowledge transfer, reduce the number of defects released, and increase maintainability and readability of the code.
The fact that you have CI seems like a non-sequitur, unless he's saying that you can only run CI checks on the mainline. For context I've spent my career at small engineering organizations, so maybe when you have 200-2000+ engineers you can't effectively manage running the CI checks on the branches into the mainline, but this seems unlikely. From the article Kief notes:
... the typical practice is to wait until completing work on a feature or story before integrating a pull request with the mainline. Most teams take longer than a day, on average, to develop a story. So a typical pull request process doesn’t meet the minimum requirement of Continuous Integration to integrate everyone’s work at least daily.
Continuous Integration via committing to the mainline branch every single day allows you to avoid merge conflicts and ensure a smooth development flow. Here he reveals the real problem though. Typical teams don't break down their work into small enough chunks that they can integrate into the mainline daily. Rather than addressing the problem of work not being broken down properly, in a way that will lead to true continuous feedback and improvement, he seems to say we can just skip code review, and suddenly the problem goes away.
Kief argues for three alternatives to pull request reviews:
- Periodic Reviews
- Pipeline Approvals
Pairing is the best alternative he presents, but most businesses are going to heavily oppose it because it looks like doubling the cost for half the work. (Research shows that this isn’t the case, but that’s for another post.) And on the engineer's side you are now tied to another person for better or worse to do your work, which may or may not be what you signed up for. This is why I find code review to be the better tool generally while advocating that engineers reach for pairing on an as-needed/desired basis.
Periodic reviews will dramatically reduce the efficacy of reviews. (See SmartBear in partnership with Cisco.) Their study shows that the efficacy falls off dramatically when you review more than ~400 lines of code per hour. I believe that focus and ability to detect issues during code review also drops off dramatically after an hour. Batching changes into a single review means really increasing the odds of releasing a bug into production.
Pipeline approvals, the Kief says are:
... conceptually similar to a pull request in that it is a gate in the delivery process, but you place the gate after code integration and automated tests. Doing this means that a human only spends time reviewing code that has already been proven technically correct.
This raises another bad assumption, or bad process implementation. Why would you review code before you've let the test suite run in order to prove that it's at least a baseline level of correct? And if you do wait for that, then you've either reinvented pull request reviews if you're doing it on every commit, or you're running into the same issues that you run into with batched periodic reviews.
In another post I'll tackle different ways to ensure you can break down your code into small enough chunks, and use feature flags in order to practice continuous integration while practicing continuous feedback in the form of code review.
Sign up for my newsletter if you'd like to read more about how to ensure fast, frequent feedback on your engineering team, powering continuous improvement.