Working with Pull-Requests
From my earlier posts, it might become evident that I am a fan of trunk-based development within small teams, if circumstances were to allow for it.
However, as many of you might have seen through your careers, those circumstances aren’t always available. A well known, common and (dare I say) battle tested alternative is the pull-request model.
The Problem with Pull-Requests
The pull-request model gets a lot of hate in certain communities, and it is not without merit, I might add. Some of the frequently leveled accusations against the working model are
- I can’t get my code reviewed on time!
- I have so many merge conflicts!
- I have to fix formatting changes on the code, before I get any feedback on my story! And there’s someone else waiting on my changes!
If you were on most pull-request models in the wild, you might have faced maybe one of the situations above, which is the best case scenario, to all of the scenarios above. The latter is more common, from my experience.
And while trunk-based development has a ton of benefits, including a better lead-time, or, catching problems early — many times, there might be no way to do this in your engagement. For example, if you have a regulatory reason for having pull-requests or are not open to code-pairing.
On a past engagement, our team was able to overcome most of the problems people see on pull-request models, by tweaking the process a bit. Some of the steps were ones we actually did on the team, but some weren’t. However, even with a reduced set of the following rules, it was the most comfortable pull-request model I’ve worked with.
Our Aim
Get the code to the master/main branch as soon as possible.
Getting There
1. Pull-Requests must be small
How small? You should be able to review them within that short span of time you get before you resume work after taking a break.
As such, the changes in an PR must be less in number, and they must be logically and functionally focused.
2. Pull-Requests can be chained
A larger feature will have multiple pull requests. Since they are dependent on each other, tweak the base-branch of subsequent pull-requests to point to the one before it.
The advantage is that this keeps the change-set in each pull-request small, and easily reviewable. You could title chained pull-requests with ‘-Part-1’, ‘-Part-2’.. to help your reviewer out when following this process.
3. Chained Pull-Requests must use Feature-Toggles
There is no point in having a chain of pull-requests if they are not merge-able to the master/main branch soon.
Often, this is not possible without the help of feature flags. A good technique is to start off a pull-request chain with a commit to introduce a feature-toggle to the code-base for the new feature, and setting it to the ‘off’ state. Remember the core principle, merge code changes to master as soon as you can. And feature-toggles can help you there.
4. Pull-Requests can be rejected only for one reason
Which is that it will break production if merged immediately. Or if merged, or might affect other development streams in the team.
That does not mean reviewers need not review for for code-readability or clarity, or opportunity to refactor.
On the contrary, you must watch out for these things. However, my strong belief is that these kind of changes can be made with follow-up pull-requests. Until then, merge all the code that you have now, and ensure that review comments are tracked in your issue tracker. This also allows you to defer non-important suggestions to technical debt, if need be.
And for the sake of time, please automate linting and source-code formatting. If it cannot be automated, it must not be a blocker for a merge.
5. Create ‘empathetic’ Pull-Requests
Pull requests should be written in a way that the reviewer has a good top-level view of the changes. I usually achieve this by clearly mentioning
- What changed as a part of this Pull-Request
- Why this change was made
- Factors considered in technical decisions
- Testing notes, specifically, the areas of functionality that I had tested
- Screenshots, if it is a UI change
Merge soon, don’t leave Pull-Requests hanging
This requires good team discipline. One must never underestimate how individual team-members might still not review Pull-Requests on time.
This, however, is a difficult problem to solve, but it is something that must be done.
In my experience, I’ve seen my pull-requests being merged pretty soon if I followed some of the practices I’ve learnt over time.
And if it still doesn’t help, and if nudges and feedback don’t do the trick, then maybe it’s time to find another team.
References
A huge inspiration for our team was the following blog post : https://hackernoon.com/the-art-of-pull-requests-6f0f099850f9
