And that's not rare in teams. Lots of teams and developers do code review wrong.
I even hear other people complain that I "block" their code review. I mean, if there are issues in your code, of course I am going to flag them, what do you think the purpose of code review is?
In this sense, I'm not sure I've ever seen a team that does codereview "right".
In the before times, most PR feedback was stylistic, with the occasional bug identified. Now that we have ubiquitous auto-formatters/linters/CI, most PR review falls into either "you misunderstood the spec", or "I disagree with your architectural choices" - and my personal feeling is that your process ought to catch both of those well before the PR stage
I think that only speaks for your own experience. I have definitely seen more than a few PRs that needed significant work.
I'm not sure how much I'd enjoy working on teams who were routinely producing PRs that were in bad shape.
On your original claim, I have seen engineers put up 5x more PRs simply because they paid less attention to the quality or put less thought on each one of them.
I have seen people put up 5x more quality PRs too. But as long as they follow the good practice of doing a code review for every PR they put up (or 2 if you require 2 per PR), they got their stuff through quickly as well.
> your process ought to catch both of those well before the PR stage
We have multiple points where mistakes of any sort can be caught, and code review is one of them.
Yes, most architectural issues should be caught earlier, but some will only become evident in code: some by the dev themselves, others by reviewers.
This is only a problem if you mostly catch architecture issues at code review phase.
The argument here is that all code reviews are done with attention and care, but quality of a code review is highly dependent on the reviewer and the team’s review process, and in the real world the quality of reviews pretty much follow the same distribution curve as, say, agile project management: For the time invested in reviewing, a handful of teams get excellent utility from them, most teams get little benefit, and a sad few actually cause harm.
If most code reviews provide only a little benefit at base for most teams, recommending that most teams should also delay shipping quality work is going to sound a lot like bad advice.
I’ve noticed that large PRs aren’t just a problem for human reviewers: they’re a problem for AI reviewers too.
If I submit a 100 line PR I’m likely to get some useful comments back from both humans and LLMs. In fact the LLM is likely to come back with so much feedback it gets down to the nitpicky/annoying level.
If I submit 1000+ lines in my PR, the humans either don’t have time and/or get scrolling blindness, and the AI reviewer is likely to give me a response that amounts to, “<<slaps roof>> Looks good to me bro: ship it!”
I guess they have a limited token budget for reviews so you can bamboozle them simply by blowing most or all of that budget.
I think the ultimate answer to this is a stacked PR workflow (which we had at Meta), where I can cheaply maintain/review a 1,000 line PR as a stack of 10 incremental PRs. But unfortunately GitHub et al are still not quite there on this one.
It is definitely very grunt-like for an LLM.
Code reviews, documentation, static analysis, only retrieving deps from internal repos, unit tests, integration tests, ....
Especially in domains where shipping software is not the main product, and a plain cost center to the main business of physical goods.