(no title)
bguthrie | 4 years ago
Advocates of pre-merge review point out (correctly) that peer review is valuable: humans are fallible, and a second pair of eyes often helps. Maybe I read a different article, but I don't think the author disputes this. What gets lost in the discourse around the dominant PR-based, asynchronous workflow is that it comes with tradeoffs. Do you understand what you're giving up to get back in your preferred mode of working? Are you so certain it's more appropriate for your present circumstances?
_Forced_ pre-merge review has a number of negative tradeoffs that aren't always visible to the teams that use it. For one thing, it can lead to "review theatre": casual pull request reviews can't meaningfully detect most bugs; reviews that can are hugely time consuming and as a result quite rare; poor PRs are sometimes "laundered" by the review process; poor reviewers encourage bikeshedding; but even a bad review can introduce a cycle time hit and a bunch of context-switching as both the submitter and reviewer bounce back and forth. If you work at a shop that uses PRs and has none of these problems, I salute you; I have not.
The answer to all of these from PR advocates tends to be, well, maybe make the pre-merge review process itself better, to which I say: you are making a slow process slower; if your team is trying to move quickly, instead of adding additional padding around a slow process, maybe try to smooth out a fast one?
A good framing question I ask my teams is: if your goal was to get high quality code into production as often as possible, what processes would you tweak and why? Where would you invest and where would you pull back? There are lots of great ways to ship high-quality code quickly without pull requests; we did it all the time before they were invented.
peheje|4 years ago