Anthony Steele

Bloggy

Against Pull Requests

The git Pull Request is a powerful tool. But just because you have it, does not mean that you have to use it all the time.

Pull requests have a powerful legitimate use. Consider a team that owns on a codebase and works on it regularly. Pull requests give outsiders a powerful thing that they did not have before: controlled access to apply changes that they need, with review by the owners. Someone can come along and say “I’m not a member of your team, but I need as an outsider to get some code into your project, so have a look at my offering.”

However pull-requests is not a good process to apply across the board to those same owners as a matter of course. Pull requests give everyone controlled access: they allow anyone to knock on the door of the room and say “Can I come in?” But if you work in the room, why would you do that?

I also think of it as like a gate in an ancient walled city: A stranger can come in, if they pass inspection at the gate. Without the gate, they would be reduced to standing outside and shouting over the wall “We need this feature!” and hope that it happens soon. Possibly an ambassador will come out to negotiate terms and times. So the gate instead allows them to come in and get business done, while respecting the local customs. But would you put that checkpoint inside the city and apply it to every citizen?

Pull request give review. There are other ways to do review. Human review is inconsistent, time-consuming, not scalable or repeatable and invariably has a political aspect. Review helps as a bug-finding process, but it is not a good substitute for e.g. simple test coverage. Review helps as a style and architecture process, but is no substitute for pairing.

Pull requests do not scale - A tailback of unmerged PRs is always a bad sign, and not just because of having to rebase or fix merge conflicts. An unmerged and undeployed PR is work in progress. Your cycle time from writing code to deploying code when joining the back of that queue is much greater, and the risks are greater too. In extreme cases there can be forgotten PRs that are weeks or months old, no-one remembers exactly what their intent was, and are neither easy to safe to merge any more. This is just wasted work.

Continuous integration (CI) is the practice, in software engineering, of merging all developer working copies to a shared mainline several times a day. https://en.wikipedia.org/wiki/Continuous_integration

Pull requests encourage you to forget that the “Integration” in “Continuous Integration” literally means “Merge to master”. A branch or PR is by definition not integrated.
You can test a PR, but you can’t continuously integrate without closing it.

If I was starting from scratch with setting up a process, I would work hard to avoid mandatory Pull Requests and encourage Trunk-based development. It might be that the lack of PRs would force you to do other things: good, those other things are things worth having.

This all supposes that code has an owner. If you don’t have such ownership, i.e. nothing but feature teams, you are going to have a bad time in the long run because of this lack of ownership.

Practices for using Pull Requests

If you are going to use PRs, consider these guidelines to make the experience less painful: