“We favor the comfort of conviction over the discomfort of doubt.” - Adam M. Grant, Think Again: The Power of Knowing What You Don’t Know
You should be pushing directly to
I don’t feel comfortable saying that. And if you are anything like me, you have recoiled at that. But now that I have seen the error of my convictions, I am going to start the uncomfortable journey of changing my habits. And I will try to share with you what convinced me to question my love for pull-requests.
Here is how it started:
Early last week, I received an email with a link to this blog by Trisha Gee. She listed 5 reasons why she prefers trunk based development. My first reaction was “Hell no! There is no way that this could be efficient.” If that sounds like I responded without thinking deeply about it, it is because I did. It was my intuition to say “Hell no.”While I generally trust my intuition, I know to question it as well. So I took a deeper look at Trunk Based Development.
While I agreed with the benefits of pair programming and how TBD can work, what triggered me about Trisha’s blog was the hostility towards pull-requests. My response to her blog sparked a very interesting internal discussion with my esteemed colleagues here at kreuzwerker. And they told me that they don’t even have a short-lived feature branch. They were just pushing to the trunk (AKA main). (┛ಠ_ಠ)┛彡┻━┻ I couldn’t understand how they couldn’t see the problem with that. I couldn’t fathom how pushing to the single source of truth without any safety check is a practice preached by our industry leaders (such as Dave Farley).
Don’t get me wrong, I have seen the treacherous roads of git-flow. I’ve seen it work for a minute and then stop working, leaving behind a planet sized monolith with no tests and a flock of QA testers required to keep it running. Even the creator of git-flow Vincent Driessen has updated the blog where he introduced git-flow to add a warning. What I was advocating for was TBD with mandatory pull-requests, which I later found out is called Scaled TBD.
I love/loved pull-requests. I use them all the time. Even for myself on my personal projects. I put my neck out to defend them. So, I started writing a blog to set the record straight, to show them the errors of their ways. The great thing about working at kreuzwerker is that I get to have my ideas challenged by my colleagues like Sharib and Kateryna. And boy was I wrong (well sort of, I can explain). And here I am writing a completely different blog than I had set out to put out.
Why did I love Short-Lived branches and Pull Requests?
Flexibility and Confidence
Anyone who knows me enough knows how much I love refactoring. I’m a good Scout. My pull requests often contain 15 file changes because I either renamed a class, reformatted the code or even upgraded a library causing a cascade of changes. I am usually worried that I might break something.
So, not only do I need to get it reviewed, I would also like to take a serious second look at it. I do a Pull Request review on my own code before I ask anyone to do that for me. I check if the unit tests are all green. I usually don’t even run integration tests on my machine unless I really have to. They are simply too expensive and often unnecessary. I push my changes and let my CI/CD service run the tests for me. I also have a quality check like sonar running as part of the pipeline. All that gives me a lot of confidence that my changes are A-OK to make it to my precious
I am also splitting the refactoring commit separate from the actual feature change. That makes for an easy communication of intent.
Clean Commit History
I like my commits to be meaningful. But you know how it goes, there is always something. I often have a lot of meaningless commits. It could be because I was doing a refactor or because I was going back and forth between tasks. My short lived branches give me the security and flexibility to just commit, switch to a different task, complete it and come back. I also use commits as checkpoints. The changes that are not yet committed always tell me what I was doing yesterday. With a PR and short-lived branches, all these meaningless commits are not a problem. You just need to do a git rebase origin/main -i (AKA interactive rebase) when you are ready to create a PR. This way, I get to fix conflicts once and without introducing a merge commit (like using Gihtub’s Squash and Merge). That is a sweet deal.
I sometimes also discover that I have a heapdump file or other unnecessary files checked in by mistake. I’m not afraid to admit that I’ve checked in my maven target directory by mistake. Checking the diff on the PR is usually how I realize my mistake and clean it up before it goes into my precious
Other than convenience, my biggest reason to love Pull Requests is that we have a quality gate. You have a dedicated place to make sure you are producing quality code that is well tested. In your Pull Requests, you can gain insight into:
- How big or small your tasks are and how well they are broken down
- How clear the tasks were
- How much effort it took to implement the feature
- Quality checks tell you how well the code was covered by tests and if it follows team practices
- Is everyone in the team contributing equally
There are a lot of reasons I listed. I did love pull requests. I still do.
So, why I changed my mind
Maybe you have already picked up on a pattern there. And you may be saying ‘duh’. But then again you could be someone like me, waiting to be inspired.
There are a few things to unpack.
Do you trust your team to follow good practices? Do you write good tests and execute them often to make sure everything is green?
Because if you don’t, then whether there is a pull request or not, it doesn’t really matter. The reviewer can be as negligent as whoever wrote the code. If I can ignore failed tests, so can the reviewer. Unless you are creating a hierarchy of reviewers, there is really no point in not trusting the team. The team functions in a system of trust and responsibility. Pull Requests create a false-sense of security that doesn’t really exist. You can and should trust your developers. Or at least cultivate a process that promotes a culture of trust and collaboration. And that’s exactly what pushing to the trunk does.
If you break the trunk, you break it for everyone. While this may slow down teams that are not yet mature, I believe it will help develop a good culture. For example, if your changes are small enough to be understood by everyone, anyone in the team will be able to help fix your breaking changes. It’s okay to make mistakes; the key is to ensure you have a way to bounce back.
Do you spend time thoroughly breaking down your tasks? Are these breakdowns meaningful and require reasonable effort? Are these tasks likely to be completed within a day or two?
Because if you don’t, you’ll have tasks that require huge effort to complete. You’ll have a task that spans weeks and sprints. You’ll end up with a huge amount of changes and even pull requests will likely just be formalities. In most cases, reviewing a big pull-request is as good as no review. You might as well have pushed it directly.
Incentivise developers to participate in the planning and task breakdowns sounds like a better strategy. Instead of relying on pull-requests, developers will work to make sure the tasks are small enough to confidently push to the trunk. Developers will have reasons to implement feature-flags and other solutions to make it safer to push to
main with confidence. A task that takes a painful week will elicit careful consideration in the planning phase. Isn’t that what we want with agile?
Do you finish work before starting new work? Do you value quality work over speed?
Because if you don’t, you’ll have a dozen works waiting for review. You’ll stack up investments that have not yet yielded, which is what pending pull requests are. Having a pull request process will likely allow developers to move on to a new work before finishing the previous task. It allows developers to offboard the task of ensuring quality to the reviewer. This, I’ve come to see, is not sustainable.
Pairing and Collaboration
This was the biggest factor for me to change my mind. If you have read between the lines, you might have noticed that I like to work solo. I do; I like to go fast and the way to do it is by myself. But that doesn’t necessarily mean going fast as a team. In fact, I’m still hungover from my last role where I realized late that I manufactured a lot that may not survive after I left. I did produce fast, but it ended up slowing everything down in the end because all of the sudden everything needed me to function. Bus factor = 1. As much as my ego liked it, it was disappointing to think that I have left a legacy that I’m not so proud of. It was a stark realization that my work may now be that piece of legacy code everyone is afraid of messing with.
And I did create Pull Requests for all these changes I made. Not always, but I did. But it was the same whether I created a Pull Request or not. Reviewing 100 lines of terraform code wasn’t something my colleagues wanted to do. They also didn’t have the skill. But the pull requests gave me the false sense of confidence.
It definitely would have been slower at first. But I now wonder how much slower it would have been in the end. How much more could we have done if I wasn’t needed to make any changes. How much more could I have done during my notice period if I didn’t have to onboard my colleagues on all that legacy stuff.
I still like pull requests, they are great in many ways. I love working asynchronously. They are a good place for quality checks and to have discussions. They are hard to replace in open source and large projects with large teams. Pull requests are still needed by teams that are yet to mature.
But I’ve come to realize how damaging they can be to the culture and how much they take away from pairing and working together. I’ve failed to realize reviews are not silver bullets. I will definitely miss switching context as easily, but then I shouldn’t have to. It will feel slow and awkward, as any change would.
I am not a fool. I realize how difficult the change could be. But here is an idea. Go without Pull Requests for a month. Force the team to pair on changes, to run tests locally and to push everything straight to main. Maybe that is all you need to start the journey, or maybe you will figure out what is blocking you from doing that.
Because, my friend, for me it was all about fear.
/* P.S: TBD is not easy. It requires maturity of teams, maturity of pipeline and even developer workstations that can run integration tests without that becoming a hurdle. It’s also easy to implement it incorrectly and then blame the practice. I, as much as I would like to, can’t offer you any good tips here. But I hope you’ve found my insights helpful in giving it a go. */