This content originally appeared on DEV Community and was authored by Conor Bronsdon
The Pull Request Paradox explained
I just wrote some code that can have a positive effect on our customers and I’m motivated to release it as quickly as possible. I need your help but you are busy and motivated to continue working on your own code. This conflict is The Pull Request Paradox.
A very, very brief history of pull requests
Pull requests came from the open source world and were born out of necessity. We needed a way to help open source projects manage submissions from contributors all around the world. GitHub built a solution for open source and Git flow went on to become widely adopted in the corporate world too. Despite the fact that most teams using it were working in the office. Of course in the last two years we’ve come full circle with remote work. So does that mean pull requests are more relevant than ever? Maybe.
But the rise of PRs means all of a sudden we have this new gate between us and merging/releasing. We used to review each other’s work in the same room. Either through pairing or sync review sessions. When we were done, we merged. Today, most PRs are completed asynchronously with a lot of back-and-forth. And that back-and-forth introduces idle time into the process that was never there before.
The Pull Request Paradox hurts developers
So what’s the big deal with idle time? It’s not like we’re waiting around for someone else to review our work. We issue a PR and then move onto the next thing. It turns out idle time, particularly in the pull request process, is a developer flow killer.
The data science team at LinearB studied 733,000 pull requests and 3.9 million comments from 26,000 developers and found that:
- 50% of pull requests were idle for 50.4% of their lifespan
- 33% of pull requests were idle for (a whopping) 77.8% of their lifespan
- The average cycle time for the developers studied was 6 days + 5 hours
- The average pull request review time for these developers was 4 days + 7 hours
This translates to an average of two days of idle time for every single chunk of work! My team struggles with this. Not only is it frustrating, it creates real productivity problems:
We can’t merge and release. Our mission is to solve problems (often through writing impactful code) and get our solutions into the hands of customers as quickly as possible. Idle time hurts our ability to merge and release our code which stops us from delivering value.
Idle time leads to reduced situational awareness, lower code quality and wasted effort. The cognitive load for revisiting our code increases every hour that passes after we open a new PR. If I get questions or a request for changes a day or two later, it’s very difficult to get back into the flow state I was originally in. Sort of like how every time I leave this blog I have to re-read the whole thing before I can start writing again 😂 Idle time hurts quality because when a problem occurs in production on code I wrote days or weeks ago, it’s a lot harder to debug.
Our team misses commitments. Our team leads already have a hard enough time with accurate sprint planning. Idle time and longer cycles make our work less predictable and cause us to miss commitments.
Want to help us fix the pull request paradox? We would love to get your feedback on our pull request promoter – sign up for the beta and be the first to get access.
Should we get rid of pull requests?
Many people have proposed alternatives to pull requests:
“True” continuous integration. It’s popular these days to say that CI and PRs are mutually exclusive. Trunk-based development allows devs to commit directly to the main branch without any kind of review or merge process. This approach might work for the most elite teams, but I think it has more cons than pros for 95% of us.
Ship / Show / Ask is defined by Rouan Wilsenach as:
“A branching strategy that combines the features of Pull Requests with the ability to keep shipping changes. Changes are categorized as either Ship (merge into mainline without review), Show (open a pull request for review, but merge into mainline immediately), or Ask (open a pull request for discussion before merging).“
I really like this approach. Not all pull requests are created equal. For low risk work, merging directly with no review or review after the fact makes sense. The problem with this approach is that today most teams I’ve seen don’t have the definitions, process and automation in place to make it work.
Pair programming: Pairing is awesome. Though it’s a bit harder now that we all work remotely. I know a lot of dev teams that use pairing to compliment async pull request reviews and I’ve never personally worked on a team that uses it to replace PRs.
I don’t think pull requests are going away. In my experience, having a teammate review your code before your merge is the best, cheapest way to increase quality and reduce bugs. PRs are especially effective at catching maintainability bugs which are hard to detect with automated tests.
I polled the members of the Dev Interrupted Discord community and found quite a few developers who agree PRs are a great tool for improving quality while learning and teaching.
The number of professional software developers is expected to double from 20M to 40M over the next 10 years. That’s a lot of new developers who will benefit from having another set of eyes on their code.
PRs have value but also a fundamental flaw. So how do we change the process to get the value and also cut idle time and merge faster? Our team started experimenting with a new thing earlier this year that’s making a big difference 👇
Promote your pull request and merge faster
For a long time I thought nothing could be done about this paradox. It is what it is. Then I ran across a research study Which Pull Requests Get Accepted and Why. The paper shows smaller PRs are more likely to get accepted. It’s proof of the old saying… Show me a small PR and I’ll show you one with multiple thoughtful comments. Show me a large PR and I’ll show you one comment that says “LGTM” 🤣
It makes sense. When I’m considering picking up a teammate’s pull request, I’m personally much more likely to pick it up sooner if I know it’s small and I can squeeze it in between other tasks.
That got me thinking… What other information about a pull request would make me more likely to pick it up quickly? And what would make me less likely to pick it up?
Every pull request has a buyer and a seller
As developers, our job does not end until we merge. And we can’t merge without help from our teammates. And our teammates have their own work to do. So at LinearB we have adopted the mentality that, when you open your PR, it’s your job to sell it to your teammates. We give as much information as possible to make it easy for our teammates to pick up faster and give feedback faster. We just spent all of this time writing this really important code, the least we can do for ourselves and our teammates is take 5 more minutes presenting our work with the relevant context.
Seller context checklist
We came up with a list of 10 pieces of context that we share to help our teammates. We would love your help adding to the list. Please comment or connect with me on Linkedin if you have any ideas.
| Less Likely to Pick Up Quickly | More Likely to Pick Up Quickly | |
|---|---|---|
| Number of files changed | I don’t know how many files have been changed or I know it’s a lot | I know only a few files have been changed |
| PR size | I don’t know how big the PR is or I know it’s over 500 changes | Less than 200 changes |
| Review time | I don’t know how long the review will take or I know it will take a long time | I know how long the review will take and/or I know it will take 15 minutes or less |
| Related issue | I don’t what project issue the PR is related to | I can see the PR is related to a hot fix, P0 bug or super important feature |
| Release | I don’t know if this PR is meant to be included in an imminent release | I know this PR is meant to be included in an imminent release |
| Importance and urgency | I have no idea how important this PR is to you | I know that this particular PR is important to you and I can help you by looking at it quickly |
| Test coverage | I don’t know how much, if any, test coverage there is | I know there’s 75-100% test coverage |
| Assignment | I don’t know why you assigned this particular PR to me | I know the specific reason you assigned this particular PR to me (e.g. I know/own this area of the codebase, I need to learn this area of the code base, I’m on another team also working on this area of the code base) |
| Pair programming | I don’t know if you want/need a conversation or to pair on this PR | I know whether or not you’re looking for a conversation or a pairing session |
| Availability | I don’t know what your availability is if I have questions for you once I start reviewing | I know specific times I can reach you so I can synchronize my review with your schedule in case I have any questions |
Benefits of promoting your pull request
Our team has been using the seller context checklist for about 5 months and we’re seeing several benefits. Some benefits are expected like we’re merging faster and most people on the team feel less frustrated by the PR process. Some benefits are unexpected like a couple of our more quiet devs are reporting it’s easier for them to get a review now because the context democratizes the process and they don’t have to bug teammates as much for help.
Another benefit is that we have a constant reminder to break our work into small chunks. Small PRs are the foundation of agile engineering but it’s still really hard to practice. We’ve been tracking our PR pick-up times and found clear evidence that small PRs with lower estimated review time are getting picked up much faster than big PRs with higher estimated review time.
And you can see over that same period of time our average PR size is trending down. Coincidence? I don’t think so.
Help Us Fix The Pull Request Paradox
We need your help! Our team is using what we’ve learned to build a free tool to help you promote your PRs with all of the context your teammates need. It includes:
Estimated review time that learns your PRs over time
Number of files changed, commits and lines of code
Field for related ticket, release and other context
A gif or meme to get their attention 😁
Tons more cool stuff coming in Q1
We would love your feedback. Sign up for the beta and be the first to get access.
This content originally appeared on DEV Community and was authored by Conor Bronsdon
Conor Bronsdon | Sciencx (2021-12-21T18:28:09+00:00) The Pull Request Paradox: Merge Faster By Promoting Your PR. Retrieved from https://www.scien.cx/2021/12/21/the-pull-request-paradox-merge-faster-by-promoting-your-pr/
Please log in to upload a file.
There are no updates yet.
Click the Upload button above to add an update.