Why Pull Requests Are A BAD IDEA

Pull requests are widely used. Most teams create a pull request somewhere on their route to releasing change. They see them as the only way to conduct code reviews, but they are not. Pull requests were invented to manage large, open-source projects, but working within an organisation with a team is not the same thing as open-source programming. In general, our advice is don’t do pull requests, but why is that, and what should you do instead?

In this episode, Dave Farley, author of Continuous Delivery and Modern Software Engineering explores the idea of pull requests and the issues that they raise. Dave offers several alternatives and discusses the pros and cons of each, ending up with some concrete advice on what you should do instead.

-------------------------------------------------------------------------------------

Also from Dave:

🎓 CD TRAINING COURSES
If you want to learn Continuous Delivery and DevOps skills, check out Dave Farley's courses

📧 Get a FREE guide "How to Organise Software Teams" by Dave Farley when you join our CD MAIL LIST 📧

_____________________________________________________

🔗 LINKS:

_____________________________________________________

📚 BOOKS:

In this book, Dave brings together his ideas and proven techniques to describe a durable, coherent and foundational approach to effective software development, for programmers, managers and technical leads, at all levels of experience.

📖 "Continuous Delivery Pipelines" by Dave Farley

NOTE: If you click on one of the Amazon Affiliate links and buy the book, Continuous Delivery Ltd. will get a small fee for the recommendation with NO increase in cost to you.

-------------------------------------------------------------------------------------

CHANNEL SPONSORS:

[sciros]

I've done full time pair programming, solo go-into-a-cave programming, and everything in-between. I like being able to work across that spectrum rather than be stuck in one mode for too long. One criticism of pair programming I have from personal experience is that there are natural "breaks" that your brain wants to take as you're working, whether on simple problems or difficult ones. And, sure, yeah, that's when you check your email, or go get a coffee, or just get up and walk around. With pair programming, you often tend to "sync" up with your pair partner, which has its pros of course, but the con is that you're not taking mental breaks when you want to (and same goes for your pair partner), so you can end up really mentally drained after some time of it, maybe even without realizing it. And that's when you, yes even as a pair, start making some pretty stupid design decisions. The other issue is, of course, that some people work much better at different times of day. I am a night owl and figure out the trickiest problems after 11pm when I get into a "flow." With lots of people, that's not the case. So I think it's a really useful mode of work but I most liked doing it _some_ of the time. These days I don't code much at all, and I do kind of miss the camaraderie and shared gratification of pair programming.

[benbaert]

From what I've read about pair programming, the studies, which are often fairly poor quality, have been generalised too much. Many of the studies (including the one linked in the description) is about junior developers or even students. It makes sense that working through a *challenging* problem collaboratively has benefits, to bounce ideas off each other, etc, but more senior developers will also do a lot of work that is within their comfort zone in their daily lives, and pair programming *all the time* seems just like a waste of resources to me and ends up being frustrating rather than inspiring. If you're a more senior developer who's doing work that's within both programmer's comfort zones, it just ends up with one person typing and the other staring at the screen, checking their phones or just zoning off.

Pair programming is valuable if used selectively, that is, when two developers work on a challenging problem, or for a more junior developer to learn from a more senior one, or to occasionally get exposure to different ways of doing things even if it's within your comfort zone. A culture that encourages pair programming in circumstances where it makes sense is great; a culture that requires it even when it does not make sense is bad.

I find statements like "always do" or "never do" to be dogmatic. Understand when certain practices are beneficial, and then apply them appropriately.

[moodynoob]

I worked with Pivotal Labs where they rigidly followed these "best" practices to an extreme. The experience of pairing 8 hours a day also traumatized me. The entire project was a shit show, and it taught me that blindly and rigidly applying these practices, without understanding the context, culture and nature of work of the organization is a recipe for disaster. I might add I've worked on WATERFALL projects with better outcomes 😂

[elguaro]

You can have builds running on PR’s which do a local merge and run tests on that. Ofc if there’s multiple PR’s/features at the same time you are not integrating anything and everything gets messy. But having tests run on a PR can give confidence of the quality of the change and assist reviewers.

[MichaelPetito]

I think instead this video should have been titled "You need to stop creating large pull requests" and we could all go on with our day having short lived branches, fast semi-automated reviews, and approval by anyone on the team (including the author, if they so choose).

[sto]

I understand the benefits and purpose of pair programming, but I find it to be a mentally draining process. It also impedes my ability to achieve a flow state of thought.

[MurariAlex]

Code review is about readability, is about answering the question "will I be okay maintaining this?". If you're involved in the process of writing the new code (alone, in pair, mob, etc.), you're in no position of telling whether or not your code is readable, because you have fresh in your mind why you wrote it that way, the requirements, the failed attempts of writing iit another way, etc. The REAL readability test is when someone that doesn't have that knowledge swifts through your code and says "I understand it". Pair programming doesn't solve that.

"One of the roles of a PR is to verify that someone who didn't write the new code can understand it. The constant communication of pair programming can result in code only that pair understands. Does a book with two authors not need an editor?" - Laurence Gonsalves

[thehuggz-ik]

I always considered one of the benefits of code reviews & PRs is that it gets someone's brain who was not actively involved in the development process into the picture. Having a pair of eyes on the forest itself, that has not spent their time wandering through the detritus can be quite beneficial. But maybe, if the 2 dev's way of looking at things are different enough, this balances itself out and one or both are able to regularly step back for a grand view.

[sarqf]

I've been doing pair programming for the last 15 years. In my experience full time pair programming doesn't deliver on its promises. Intermittent pair programming (as in: at most a couple hours a day) works a lot better for me and I've seen teams using that approach be a lot more productive than dogmatic full time pair programming teams

[ArchStalker]

Nice video with some very interesting points. I have been on the waiting side of a PR many a times and it is frustrating.

However I do wish to emphasize one point about pair programming: it does not work for everyone. I know you mentioned this in your video, which was great, but I just want to bring awareness to one particular group of people with whom it most likely won't work: neurodivergents. I have ADHD and also some traits from the autism spectrum and for programming point of view this means that I have immense powers on concentration, but it requires a very strict set of parameters to make it work (no interruptions, music, freedom to work at my own pace). Pair programming pretty much disrupts all of those. I can do it in small bursts, but even just one entire day is a trial. I can see the benefits in pair programming and I'm sure it's a great way to work for many, but there are some of us out there that can't work with it, not because we don't want it to, but because our brains are wired differently.

[sp-niemand]

For me pair programming is extremely tiring. It's continuous communication, the direction of which switches sides periodically.

Besides that, I just can't enjoy working while there is a person watching every LoC over my shoulder.

Pairing / Teaming up to solve a specific problem fast and efficiently, in 1-2 hours, sure. But doing it full day every day? That's my nightmare right here.

[archibaldbuttle]

I found this video very disappointing, and is putting forward an approach that in my experience is not practical in most real world circumstances.

As I see it, the only scenario in which eschewing PRs in favour of pair programming can work is for a single relatively small, highly experienced, team working on a product. If that's your circumstances then great - follow this advice.

If you have a codebase that has dozens of developers of varying ability contributing though, I fail to see how this can be practical. The trunk will be a constantly moving target, the code that developers are working on will continually fall out of date, bringing with it the dangers and costs of merge conflicts with developers bringing their codebase up to date with main. Yes there are ways of minimising those risks, but generally only with careful coordination and planning in advance of changes being made. Overall, this feels like a nightmare to me.

As others have pointed out, pair programming isn't a magic bullet to ensure high overall code quality or potential issues with bad actors. Two poor or mediocre developers aren't going to magically transform into a good developer.

The main problems raised here with PRs seem to be around wasting time. In my experience, there are several ways to mitigate this. Firstly a developer who has raised a pull request should never just sit around waiting for reviews - they should move on to another task. CI should run on PR branches that are waiting to be merged, and CI tooling should ensure that there is sufficient test coverage, that coding standards are being is followed, etc. Ideally CI pipelines should be very fast, running tasks in parallel. PRs should be as small as practical, allowing them to be reviewed quickly, and team members should prioritise reviews.

[FLAMESpl]

You said that one of pull requests disadvantages is wait time for integration testing. Why wouldn't you do it on a feature branch? Our company is doing it all the time. What I understood is that you prefer merging everything straight to mainstream and review it later, but if such commit does not qualifies to project's standards or is wrong in a way or another, it can be hellish to revert it back, as other people adds code on top of it and starts depending on it.

[neildutoit]

I used to tell my boss to watch your videos cus they kept making me do feature branching. They watched your videos but didn't get it. Now I'm a boss and I tell my employees that they have to watch your videos cus they want to do feature branching.
Please keep making content.

[sauravprakash]

Pair programming is great but I don't think it can replace PR reviews. Esp with a remote team in different time zones, its not sustainable enough to replace async PR reviews.

Nice video! In our team we do a combination of pair programming and also have some rules that help us do pull requests-code review more effective, as:

- Two daily slots to do Code Reviews 10-15 min each
- Keep PRs below 200 lines of code
- Follow the team norms/best practices for clean code
- When a comment becomes a discussion get on a call with the author, it’s often easier to explain on a 5 min call instead of back and forth
- When a comment requires a refactor, create tech debt ticket to address it and move on

Also, we have learned that some code is candidate for a 15-30 min discussion session before start coding, taking this early feedback usually speeds up review, code/feature familiarity while keeping good velocity in change lead time.

[BradleyArsenault]

Pull requests work very well for small teams. The problems of one person's code not working with another person's code - those just don't happen in small teams. Continuous integration addresses a scaling problem. The problem doesn't occur at small scales. And pull requests have other advantages which can enhance communication and make code review way way more convenient

[myronww]

Great CI systems will build your PR in a qa branch so you can see the results withing a couple of hours or you push your changes to a private copy of the repo and the CI system can build and deploy from a private repo. Once you have a build with test results you can do your PR with test results attached. Either way, you get immediate feedback on your change within the time it takes to do a build and for the test job to make it through the queue. So you can create a pull request and that will trigger your code to get built and run. Then if and when you find out your code works, then you request a review of your PR

[hhappy]

This appears more a reason to stop enforcing Pull Request *Reviews* than removing Pull Requests totally. Pull requests are a nice way to
1. enforce linking to a requirement/work item,
2. ensure that it builds, passes unit tests, and static analysis on another machine than the developers
3. act as a trigger to run integration or end-to-end tests in an on-demand disposable 'build/validation' environment

Dave: I'd love to hear your thoughts on low-code platforms (e.g. Microsoft Power Platform) where the development loop is more convoluted and cannot be tested on a developer's machine.

[WilsonMar]

[13:56] "I prefer a conventional code review where we have a two-way conversation" (over async pull requests).