PRs in Trunk Based Development.
Wait, PRs in Trunk Based Development? Yes, PRs in Trunk Based Development.
I talk a lot about code reviews, devex, branching models and everything around them, as I’m the cofounder at pullpo.to/phunt.
One common feedback that I’ve got many times is:
Code reviews through pull requests were made for open source. In proprietary software, we should do trunk based development (implying pair programming and committing directly to trunk)
I don’t totally agree with this claim. I’m going to explain why. To give a little bit of context…
What is TBD (Trunk Based Development)?
Basically, what TBD says is that we should avoid creating long-lived branches other than the main branch (trunk). Developers should push new commits directly there or merge short-lived feature branches (will come back to this later). The only rule for every new push or merge to trunk is to not break the build. The CI must pass.
The main concept behind TBD, which I love, is that we should keep developers close to a production environment or its closest equivalent, the main branch. Compared to other obsolete branching methods like Git Flow, TBD increases ownership and responsibility among developers, which is good. It also allows us to be always ready to deploy, and as a consequence, it normally increases deployment frequency. Since we can reach production faster, it also substantially reduces cycle time and time to recover from failures.
All of this is great, but keep reading please, the interesting part is about to come:
PRs and code reviews in TBD.
If you are committing directly to trunk, the only way to do a code review before the code gets to trunk is by pair programming. There is no alternative. If you want to do a code review (and, most of the times, you want to do a code review) you need to do pair programming.
But TBD is also compatible (and actually recommends) doing PRs and short-lived feature branches. I know some people who wont believe this, so I’m attaching an screenshot from the official TBD website. https://trunkbaseddevelopment.com/short-lived-feature-branches/
TBD also recommends pairing, as it has a lot of advantages: better communication, shorter feedback loops… The thing is, pair programming doesn’t work for everyone, all the time, for every task.
In my case, I love pairing for problems that require creative solutions, or when debugging, or when onboarding a new developer. But I often feel much more productive when working alone. Many times I cannot achieve flow state while pairing.
Other people are always super productive while pairing, for every type of task, and they love it.
We are humans. Humans are different.
If you create short-lived feature branches now you have both options for code reviews: pair programming and conventional code reviews. You can detect good opportunities for pairing and go for it. In the other cases, go for conventional code reviews.
Other benefit of creating short-lived branches:
Before pushing a new commit to a branch, first you have to pull the latest version of that branch. Depending on different factors this command can take time.
Now imagine 50 devs working on the same repo constantly pushing commits to trunk. It may be difficult to find a time window where no one pushes and you can pull the latest version and push your code.
This problem is solved with PRs and merge queues. GitHub has its own merge queue feature.
My take.
- If everyone in your team is happy with always doing pair programming AND this is not a problem for hiring new devs AND you don’t have issues with the CI or with finding a long enough time window where you can pull and push with anyone changing trunk, then great! Pairing and committing to trunk is the best solution. If this condition is not met…
- Do short-lived feature branches, but they have to be short-lived. Short-lived means max 2 days and only one committer (1 dev or 1 pair programming duo). This is important. Otherwise, you are not doing TBD.
- Learn how to identify good opportunities for pairing in your team. This is a try and error thing. Every person is different. Know yourself.
- Set up you CI to run on PRs. Pair programming duos can have the option to directly merge (or send to merge queue) to trunk if the CI passes. Individual devs may need 1 or 2 approvals before merging.
I’m offering code review workshops at companies.
If you liked this post and want to implement best practices around code reviews at your workplace, let me know! I’m offering workshops and consultancy services around code reviews.
Send an email to marco@pullpo.io.
If you didn’t like the post, let me know too! I love discussing about this topics. I’ll read the comments, or send an email too.