0 days since hearing someone say "pair programming is not enough, a pull request has to be made anyway so someone fresh can review the code!"
PR is waste. PR is too late. PR is too little. Reviews after the fact are fine, but should not be blocking, unless someone worked solo. "Fresh eyes" is a fallacy and mostly offers nothing else than manual nitpicks.
#Software #SoftwareDevelopment #PullRequest #CodeReview #PairProgramming #Mobbing #MobProgramming #Teamwork #TrunkBasedDevelopment
This incident has prompted me to take my 6-year-old draft post on pull requests and finalize it in response.
#PullRequest #ContinuousIntegration #Software #SoftwareDevelopment #Teamwork #CodeReview
@ojrask is that, do you think, due to unwillingness, peer pressure, time pressure, disinterest, unwillingness of reviewer to learn/improve, badly received by author, or something else?
I find that you cannot discover everything as easily by staring at code, e.g. structure and error handling are easier than intimate mathematics or data-manipulation. But I do see value in it both for the result and best practices.
True, it doesn't work as the only solution to catch everything.
One cannot inspect quality into a finished thing. Quality and correctness needs to happen earlier than a quality control gate.
It is at best a way to share some knowledge and suggest minor changes, and at worst a rubber stamp used to play quality and safety theater with no benefit to anyone.
I would suggest trying out co-creation patterns (pairing, etc.) and trunk-based development as a default, while using PRs where actually applicable (e.g. open source, external contribs, etc.).
> One cannot inspect quality into a finished thing. Quality and correctness needs to happen earlier than a quality control gate.
Well, that's nonsense, and I think you know this too. For one, it's not about adding quality with your eyes. It's about discovering potential issues thus failing to pass the gate. Second, PRs need not be a finished work. There are draft PRs and people make use of PRs for the benefit of testing. Third, even in a finished results there can be oversights, ..
@ojrask .. usually because of missed code-paths or obscure structure/logic. Your assumption that a finished work must also be a perfect work in every way, is bad. (Three arguments. I didn't try to be exhaustive.)
Also, to note: I know about pair programming. That's not what I'm addressing.
> thus failing to pass the gate
And thus resulting in rework, meaning the quality has to be rebuilt into the thing under review from an earlier point of time, and then pushed into the QC gate again, possibly prompting even more rework. Wasteful!
Your other arguments are just bargaining for reasons to make use of PRs for things that do not absolutely require PRs in order to be done well.
@ojrask I get the point you make, but it isn't *rework* if it wasn't fully finished. It's incomplete according to the expectations set by the quality gate. There is e.g. additional work, reconsideration of design, alignment, etc.
Is it wasteful? Or is it continuation? Do you consider it wasteful to have to fine-tune your understanding of the proper solution? (Or, a later step: fine-tune a design?)
However, I think the original claim was for the value of reviews, right? The PR is a moment.
Otto Rask
cobratbq - cranky-by-design