Share code rather than pass pull requests

This is the 16th post from my sincere dev manifesto series.

There’s been a lot of traction lately for trunk based development vs pull requests. I’ve also been wary of pull requests for a long time.

A long time ago, in subversion days, there was a product called crucible. It allowed to do pre-commit or post-commit code reviews.

  • Pre-commit reviews were basically the same as what we do today. The code waited for approval before reaching the mainline.
  • Post-commit reviews were done directly on trunk (i.e. main or master branch). You selected the commits you wanted to add to a review, and off we went for an asynchronous discussion.

Believe me, the incentives from post-commit reviews were a lot more in favor of flow than phase gate pre-commit reviews.

Since then, open source became the model everybody could see. In open source, it is perfectly relevant to tightly filter what you allow in your code base. You generally don’t know the person who’s proposing a PR, and you don’t want to be the victim of a supply chain attack, or to see your product downgraded.

Almost nobody knew about post-commit reviews when git, git flow, or github, took over the word. So it went to limbo. Everyone knew the agile manifesto. Some of us even knew its principles. XP had been around for quite some years. CI/CD was seriously making its way to mainstream. Yet, git flow and PRs were such a tsunami that no one looked for alternatives.

But within a team, you want reactivity and flow. And phase gates are their worst enemies. We all know the downsides of phase gates:

  • less reactivity
  • less trust
  • less initiative and autonomy
  • less boy scout rule compliance. When there’s a risk someone else is working on the same piece of code, we don’t rename things, move code around, split files, etc, to avoid painful merges. And the longer dev is parallelized, the higher the risk of touching the same code in parallel.
  • more context switching and waste

And these are exponential when branches last longer.

Trunk based development (TBD for short) is the solution, and it’s pretty simple: push at least once a day to the main branch. You can’t do that if an asynchronous review is in the way of merging your code for hours or days.

TBD requires tooling, like careful data model and contract management, or feature toggles to isolate unpolished features from users. It requires some continuous effort. But the alternative, putting code in a fridge until it is perfect and the environment is ready for it, is even more painful.

Hey but if the new behavior is hidden behind a feature toggle, why pushing it to the main branch? Because at least, you share the same code with your team mates, it is compiled, packaged, automatically checked, and that’s already a decent part of lowering the risk and encouraging continuous code improvement. Besides, it’s quicker to rollback a change by deactivating a toggle than reverting a commit and going back to the CI/CD pipeline.

More that anything else, TBD requires trust. You need to trust devs to push code that will not violate your team’s constraints. Code quality and patterns, of course, but also performance, security, you name it. And that’s exactly what a team is supposed to be: a bunch of people trusting each other’s work. So we need to help all teammates step up quickly and share the same feel for our constraints.

Hello collaboration. When you want faster homogeneity in understanding, there’s only one tool: working together. Ideally, work as much as you can as an ensemble, the whole team on one keyboard. If you can’t, rotating pair programming is a decent alternative. If you can’t, well, you will need explicit, clear, strong, and evolving rules for asynchronous understanding, with all the negotiation, time, and effort it implies.

A team shares more alignment than a department. This is why we call it a team. It is made to collaborate daily. So why splitting it into individuals by building walls between them?

By the way, if you really want to stick to pull requests, I can at least share my rules for accepting a request. I try to avoid blocking code as mush as I can. So I click the green button if the product is at least slightly better than before without increasing the risk significantly. If the new code brings any benefit, if it does not significantly impacts performance or security of an existing feature, even if it is not pixel perfect, then I validate. The code is an imbalanced evolving creature, so so be it.

I commit to encouraging synchronous collaboration with my team, rather than abide to passive agressive phase gate pull requests.

Advertisement

One comment

  1. Pingback: Sincere developer manifesto | AAAgile

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s