(2021-04-01) Are Pull Requests Holding Back Your Team?

David Masters: Are Pull Requests Holding Back Your Team? Pull requests are great for open source. But they can hinder the team performance.

The branch and pull request workflow has also been adopted by swathes of commercial software teams, and it seems to have become the de-facto standard of development workflows.

Are we forgoing the benefits of “real” Continuous Integration (CI) in order to accommodate pull requests?

Why We Wanted Pull Requests

Pull requests have an obvious appeal. They provide a really convenient view of new code side by side with the current code.

Modern Git-based systems (such as Bitbucket) also provide the ability to set up merge checks against pull requests, requiring a successful build before it can be merged.

I Got Sucked In

after spending a few years using this approach, and acquiring newfound aspirations of achieving Continuous Delivery, I’ve started to realise that they come with serious drawbacks and maybe that they’re not actually beneficial in commercial teams.

If You’re Using Pull Requests, You Probably Aren’t Doing Continuous Integration

Continuous Integration means something very specific.

Everyone commits to the mainline every day. (Trunk-Based Development)

So in a team that uses pull requests, unless everyone in your team is merging at least one pull request every day, can you really claim to be doing Continuous Integration?

No Dogma Here, BUT….

I do think that teams using pull requests are unlikely to be abiding by this fundamental rule, and therefore compromising what makes CI so powerful; the sociotechnical system it produces.

Feature Branches vs. Trunk-Based Development

You’re probably thinking that my argument so far is not really about pull requests but rather branching strategies, i.e., feature branching vs trunk-based development. That is true; it is the shift towards feature branching that compromises CI rather than pull requests themselves, but…

Do Pull Requests Encourage Long-Lived Branches?

the workflow has encouraged me to get everything done in one hit: a nice, big, fat squashed atomic commit that encompasses the entirety of a sometimes sizable piece of functionality.

Long-Lived Branches = Lower Team Performance

this is proven by the research and data produced in the State Of DevOps reports (SODR) and presented in the fantastic book Accelerate: “Our research also found that developing off trunk/master rather than long-lived feature branches was correlated with higher delivery performance.” (DORA)

Pull Requests Inadvertently Create an Environment of Distrust

They are a gate-keeping mechanism for onboarding code from strangers

I’ve witnessed PR comments being fuel for tension and mistrust that I believe wouldn’t have been the case if communicated via a different medium, such as face to face or in a private IM. Obviously, this depends on your team, but if there are soured relationships within it, I don’t think the PR workflow is likely to help build bridges.

“In an environment where high trust is possible, adopting processes that require trust will help create and retain it.” — Giles Edwards-Alexander

Pull Requests Aren’t Actually That Good for Code Reviews

When reviewers aren’t exercising diligence, PR approval becomes somewhat of a pointless tick-box exercise. Comparisons can be drawn with a bad Change Approval Board (CAB) process, something which Jez Humble labels as “Change Theater.”

Avoidance of Accountability

burdening a single person to review and share accountability for the code disincentives them from picking up the PR to review. Not only that, but it requires a developer who is working on their own stuff to stop what they’re doing and context switch to review the PR — another disincentive.

Lean Thinking?

I think this is a false premise. If we were really building quality in early, we’d be reviewing the code as it was being written — not waiting for developers to come out of their period of isolation and then check it.

The PR workflow generates wasted time by the potential of multiple rounds of comments and additional commits (if you’re lucky enough to have a diligent reviewer).

The asynchronous workflow makes complete sense in the open source world as people are often maintaining projects in their spare time.

Don’t Hold up the Project!

Another negative experience I’ve encountered with PRs is their relationship to projects with outsourced development teams. This is where an outsourced team has raised a (usually huge) PR, and the in-house developers are tasked with reviewing the changes

So if Pull Requests Are Bad, What Is Good?

I think that the practice of Continuous Integration (in its intended form) combined with Pair Programming/Mob Programming is likely to yield the best results

Maybe Pair/Mob programming should be renamed to Continuous Review.