deddit.petersanchez.com

Stop Using Pull Requests (a4al6a.substack.com)
from codeinabox@programming.dev to programming@programming.dev on 22 May 11:39
https://programming.dev/post/50793895

TL;DR

Pull requests were designed for open source contributions from untrusted strangers. Applying them to trusted teams is a category error.
Peer-reviewed research shows code review’s primary value is knowledge transfer, not bug detection. Less than 15% of review comments relate to actual bugs.
Async PR workflows mean your code spends 86-99% of its lead time waiting. One organisation spent 130,000 hours in a single year waiting on PRs that received zero comments.
DORA research across 36,000+ professionals shows trunk-based development correlates with dramatically higher software delivery performance, and faster code reviews alone improve performance by 50%.
The alternative is T*D: Test-Driven Development (build quality in), Trunk-Based Development (integrate continuously), and Team-focused Development (review during creation, not after).
The transition is gradual: optimise PRs first, adopt Ship/Show/Ask, then move to pairing and trunk-based development as trust and automation mature.-

#programming

threaded - newest

felsiq@piefed.zip on 22 May 13:32 next collapse

For anyone else who might get clickbaited by the headline: this is not meant for open source, they’re only talking about small corporate teams

codeinabox@programming.dev on 22 May 13:47 collapse

Thank you! I’ve updated the post with the TL;DR from the article.

karlhungus@lemmy.ca on 22 May 13:57 next collapse

disagree, pr’s catch tonnes of shit, knowledge transfer is useful even in trusted teams. they also don’t prevent any test driven development or continuous integration.

team focused development pairing are also pulling studies from 2005 before pr’s were meaningfully a thing.j Also as someone who’s done a significant amount of pairing, it’s not that easy. I can’t imagine mob programming.

NotSteve_@lemmy.ca on 22 May 16:41 next collapse

I hate having to harass people to review my PRs too but getting rid of them entirely seems insane to me. Most of the issues with pull requests can be sourced more to the size of changes rather than their existence. If you can keep your PRs small enough that someone can review it in 5-10min then things get a lot easier for everyone but deleting the step entirely means nobody is watching what’s being submitted

Like, you might have someone working on something in a new codebase but misunderstanding the architecture or just going in the wrong direction in general and a review is the best way to correct course before getting too far in

FizzyOrange@programming.dev on 22 May 17:32 collapse

I agree. What we need is better PR workflows, not getting rid of review entirely. That’s dumb.

qprimed@lemmy.ml on 22 May 17:03 next collapse

not a professional programmer, by any means - but PR’s (for me) slow development down to a point where code matches the philosophical/algorithmical goal. when code development is at the proper pace, it dramatically reduces the need for refactoring.

edit: upvoing this post, because the comments are particularly interesting - as with any “good” post :-)

FizzyOrange@programming.dev on 22 May 17:37 collapse

Yeah if you saw the quality of my coworkers’ code you would not be saying this.

TehPers@beehaw.org on 22 May 18:37 collapse

This. I can’t get them to run lints or tests on their own, and I can’t get the person in charge of the repo to let us run the CI automatically on PR. Combine that with the rampant slop, and a good number of the PRs are just plain unreviewable.

Then you run into the other issue: the PRs get merged too fast to review them properly. How someone approves 50 changed files in a PR with +30000 and -150 lines changed in under an hour is beyond me, to be honest.