Pull Requests are Just Fine, Thanks
If you search around, there's a lot of anti-PR sentiment to be found on software engineering blogs, but if you follow onto forums it doesn't seem to be a popular sentiment. Most of these criticisms are misdirected; in fact, pull requests are just fine.
There's a degree of popularity around claiming that pull requests are bad and we shouldn't do them. I'm not sure that this is the majority opinion, but it's one that over the years has been popularized by some notable voices like Dave Farley, eched by a host of easy-to-find blog posts about the their disadvantages or their dysfunctionality or even how they are hated.
A review of these articles (and plenty of others which crop up from time to time) reveal a common list of gripes with PRs:
- PRs give too much power to reviewers; open to exploitation
- If branches live long there are merge conflicts
- Large PRs are difficult to review
- If the team only gives superficial reviews there's no point
- PRs reduce or remove two-way communication (async over sync communication)
- A review at end of dev cycle is too late
- PR lag increases the amount of work in progress
- PRs make the development feedback loop longer; shorter feedback loops are better
- Sometimes it's easier to fix a bug than to explain the fix
- PRs discourage continuous refactoring
- PRs lead to negative emotions; reviewers might not be emotionally tactful
- Adopting PRs for the sake of it makes purpose unclear
These complaints fall into a couple of buckets: there's problems with power dynamics, poor practices on part of reviewers, and poor practices on the part of authors. Then what to do instead of a PR? Farley would have you do pair programming before merging your code into main, while others would do away with the mandatory code review altogether. These are the practices we used before the PR came around - like many of these commentators, I did not use manual PRs for the first years I was a software engineer. In fact, I think it was almost a decade before I worked on a team which adopted the manual review.
One thing I try to espouse on this blog is a way of thinking conditionally about different options we encounter as software engineers - whether that's process, architectural, or whatever decisions. In this instance, I don't want to say "always use PRs and never use push-directly-to-main," instead I'll want to give reasonable and meaningful conditions as to when one is preferred. I don't get the same sense from most of these anti-PR articles. Sure, they do pay some service to cases where PRs are preferred, but this is superficial and unserious. The suggestions that PRs are only ever going to be useful in open source projects or when all members of a team seriously distrust each other are ridiculous and unhelpful.
When will I suggest you use PRs? Whenever you want. When to use some alternative type of code review? Whenever your team wants. When to not review at all? Whenever you want. I've developed with teams adopting all manner of schemes and at the end of the day the software all ends up getting written. Usually. The only caveat, as with all process things, is to make sure you're measuring it and able to identify if a process is holding you back. Don't be afraid to experiment with something different!
Our industry seems to have adopted (from my perspective, at least) mandatory PRs as the default way of doing code review. I'm perfectly fine with that; it's unfortunate when an organization might prohibit teams from experimenting otherwise, but that's not a hill I want to die on. Personally, I like PRs as a default, and I think the the anti-PR claims I enumerated above are well-overblown. In fact, I like PRs a lot, and I want to respond to those supposed disadvantages and argue in favor of PRs. I'll start with the last bullet I listed: we probably shouldn't be adopting processes which we're unable to justify, so if your team only adopts PRs "just because" then by all means have a rethink on that one.
Why PRs Aren't That Bad #
There are a couple points made on the topic of power, that is, ways that PRs are perceived to give too much power to reviewers. When a comment is blocking a PR that shouldn't be blocking one is an example of such a power dynamic, but the more subtle one is when a very influential member of a team leaves a suggestion which - wink wink nudge nudge - isn't really just a "suggestion". These are concerns with team dynamics though. I know engineers tend to be a less social segment of society, but has the author of the PR with the blocking comment tried talking to the blocking reviewer? If PRs were not used would the very-insistent suggester still find other avenues for their so-called suggestions? Any process - PRs included - adopted by a team with unhealthy power dynamics will be brought down by those dynamics. PRs aren't more or less susceptible to this.
Two other problems deal with PR size: large PRs are difficult to meaningfully review and long-lived branches tend to have more merge conflicts. Indeed these problems can come up, and if this is a sticking problem for your team it can be time to experiment with something else. In order to prevent this problem, branches should be very small and very short-lived. Indeed, plenty of features take a lot of code to implement, so the strategy is to PR frequently with the smallest possible changes at any point. Adopting a culture in the team to integrate changes at a fine-grained level like this is important not only if you're using PRs. In fact, any process will benefit from frequent integration. In fact, if you happen to be on a team where each engineer prefers to work on large, discrete features in isolation with infrequent integration, no process will help you here.
If you're frequently engaging reviews then - as you should be doing whether or not you're using a PR process - you're solving two other issues brought up with PRs: the feedback loop is going to be much shorter and the reviews aren't only going to be coming in at the end of the dev cycle. Indeed, opening small PRs that are easily-reviewable thoughout the dev cycle ends up being a good way to engage colleagues on feedback. The other half of this is reviewer etiquitte - our teammembers need to be diligent about providing timely, meaningful reviews. If I'm only ever asking them to review a few files though, I find that this ask isn't just easy but indeed one to which my colleagues are eager to respond.
Continuing to adopt a healthy team culture will solve more of our problems with PRs and, since the issues with team culture aren't actually about the PR process, will help to solve the same problems in any non-PR processes. With frequent, small PRs that are easily and eagerly reviewed, there is not a PR lag which results in more work being in progress. Indeed, as colleagues I find that we do want to give good feedback, so having removed the burden of giving feedback more often than not I find that I get that feedback. It certainly can be a problem with PR processes that they lead to superficial reviews, but any process with reviews is susceptible to this in different ways. Whatever your process, identify how it might discourage meaningful reviews and address that. Oh look, we've crossed off another bullet!
On that point, I'm also not sure that a PR process necessarily leads to more hurt feelings than any other process. Any negative emotions I get from PRs are, in my experience, the same as what come up from other points during development. They're mitigated by communicating with colleagues; keep in mind that the PR comments section isn't the only communications channel available to us! So i'm also curious why it's felt that PRs discourage two-way communication: if you're not communicating enough with colleagues then you should ... communicate with them? If they rebuff you this is a larger contention for a broader group. Certainly if you identify a PR process as necessarily causing this in the team, by all means try something else. I just mean to be skeptical that the PR process is really the root cause of this?
I've left two negative points about PRs that I think are actually good points. First, I strongly concur that it is sometimes easier to fix a bug than to explain it. Does your team care about documenting fixes? If not, and if the team is comfortable with members pushing fixes with some other process, then I can certainly see how a PR process is an impediment. I don't think most teams are like this though; personally I very much value documenting bugs and their fixes, and I think that bugfixes are the best problems to get a second set of eyes on. In this case, the explanation issue isn't necessarily a problem with a PR process seeing as you're going to need to provide the documentation to someone anyway. It's admittedly difficult for me to imagine a scenario where I want to push a bug fix wihout having explained the problem.
I have observed it before that a PR process has discouraged continuous refactoring, though to be clear this is a problem with any process which includes a manual review, even the pair programming alternative from Dave Farley. The solution I have is the same as can be applied with any non-PR process, which is to encourage refactor and carve out a space for them. In a pair programming process, maybe allow refactors to just be pushed. In a PR process, I encourage my teams to allow "boyscout" PRs, which I named after the Boy Scout Rule. The team treats these PRs specially and celebrates members who open such PRs. The broader point is that any process adopted by a team can encourage negative behaviors, so no matter the process your team should identify those patterns and alter the process to mitigate, eliminate, or reverse those.
Why PRs Are Good #
As we can see, most of the problems brought up about PR processes aren't really problems with PR processes, but problems with communication, interpersonal dynamics, and the like. The same sorts of problems exist in any process, and while they might express themselves differently given different processes, I don't know that you can say they're worse in any particular process. This is a problem I have with a lot of these hot takes: they would be more valuable if they actually addressed the root cause of the experiences described.
The same should be true of any praise; if I want to convince you that PRs are good I should cite some unique advantages of them. I could say that I like PRs because they reinforce a code review culture, or because I like getting reviews from colleagues, but those are benefits of mandatory reviews and not PRs. What are the specific things that a PR process is good for? Well, this is easier asked than answered. Try to come up with a list of benefits in your head, then go over those and see if they're really benefits of pull requests specifically?
I think the unique benefits then are all going to be related to tooling in some way. Most development tools familiar to most engineers are set up really well to work with pull requests. GitHub, GitLab, BitBucket, Azure DevOps, and plenty of others have really robust PR interfaces. Take any popular IDE and it's bound to have some level of integration with these PR features. The tooling and familiarity barrier with PRs are both very low, and this is an obvious benefit. If your team is looking to some way of doing mandatory, pre-integration reviews, pull requests are fantastic because of the ease of adoption.
The tooling benefit extends to integrations with other tooling activities, too. Pull requests and, crucially, the tooling which supports them, are great at isolating specific changes and relating them to issue tracking, build automation, and documentation repositories. Being able to integrate so many parts of the tooling with the tooling that supports the code review process is a huge advantage over alternatives.
Tooling is necessary to support PRs, I think. I'm not quite sure what a PR process without PR tooling would look like (but comment below if you've got thoughts on that). The tooling itself then has unique advantages facilitating communication and knowledge sharing. In certain contexts asynchronous communication is required, and if that applies to your team then pull requests offer a pretty good mechanism for async communication on the code review or even general convos directly related to the codebase. PR tooling also makes it incredibly easy for engineers or other observers to see what's going on in the system and the development process. Indeed there are ways other than PRs to achieve this, but the unique offer is the ease of getting all of it.
One of my favorite benefits to PRs is a documentation aspect. When I open a PR I have to write out what I've done, then any conversation related to what I've done is written out in the PR until the end of time. Or, at least, until the end of my repository's life. It's a great way to be able to retrace steps later to get both the what and the why. PRs aren't the only way but, I think, the best way to achieve that benefit, at least with the current state of our tooling.
Should I Do PRs or Not? #
If you like them then use them, and if you don't like them then don't do them, by all means try something else! I want to get across that the bulk of "PR bad" articles floating around the internets aren't a meaningful contribution to the discourse. If the issue is with mandatory pre-integration reviews, then pin the blame there. If the issue is with interpersonal or team dynamics, then address the solutions in that space. That sort of analysis is what helps us make proper decisions about the conduct of our work. This popularity of terrible arguments creates a mini cacophony of almost-entirely-misdirected anti-PR sentiment - who is that good for?
PRs are fine. In fact, they have plenty of benefits which they share with the basic practices they're built on: feature branching, mandatory review, pre-integration review, and the like. PRs uniquely excel at some of those shared benefits, and the state of tooling decidedly sets PRs apart as the best way to get those benefits. Do those patterns and benefits apply to your team? Maybe so, maybe not. If you're on the fence about PRs or any other review process, I can't recommend enough Martin Fowler's writing on the subject, in which he does a great job framing PRs in the context of his branching patterns. I'd be curious in a similar analysis on review patterns, come to think of it.
Maybe in 10 years the next GitHub will have come out and revolutionized tooling in some way that something other than PRs are so easy to adopt. Maybe it will offer the same benefits in integration with other tooling, communication, visibility, and documentation, or maybe it will offer different benefits that make it a great thing to use. That's a fun thing to consider - do I actually need the benefits of PRs? Am I being impeded by feature branching or mandatory review?