I'm sure they exist in the working world, but 5 years in across 4 different companies I've yet to encounter it (beyond what I've tried to provide to others in my code reviews on their PRs).
I'm sure they exist in the working world, but 5 years in across 4 different companies I've yet to encounter it (beyond what I've tried to provide to others in my code reviews on their PRs).
1. Differentiate critical comments vs opinions/nice to haves. Not every comment needs to be resolved as part of that specific PR, they can be addressed in a follow up after it is merged in a disciplined team.
2. Be okay with reviewers contributing to the PR themselves. Some developers just get too caught up on 'ownership' of code that makes it in, when the team is responsible for it in the end(when you have a blameless/improvement focused culture at least). I don't need the original branch owner to fix a typo or nitpicks when it will take me less time to fix it. I haven't found any code suggestion features that are amazing for this, so sometimes you'll end up with back/forth disagreement and have to rebase a commit out, but most of the time it's fine and overall improves velocity. Basically turning the PR into async pair programming.
i.e. I haven't walked into an environment with a lacking PR culture and managed to effect a cultural change (yet). Maybe seniority is a prerequisite?
Still, bikeshedding occurs but to a lesser degree and over more substantial issues, like overall module design.
First, part of the benefit of peer review is that two people have seen the code and have familiarity. Even if it's rubber stamping, if someone has actually looked at it, LGTM might be enough.
Second, even if PR only catches something 10% of the time, it catches the most obvious bugs.
In a formal review you print off the code, then put 5-10 other developers in a meeting room to look at it together (no computers in this room, though sometimes you would go back and print off some more code for context). You could do about 10 lines of code per hour this way, and the developers burned our after at most 2 hours per day, so this takes a very long time. However reports were this resulted in the most useful reviews/fixes to the code.
I've never seen the above done in practice, just heard about it. Someday I'd like to see it, but I doubt I ever will.
When you are shipping code that you won't be able to update easily (i.e. cheaply), you try to remove as many defects as early in the process as possible. CI/CD and web delivery seem like they would blunt the motivation to go through a Fagan review for most software developed these days.
I would imagine that some form of this still happens in safety-critical systems (or more likely the state of the art has advanced from this).
... and it has the added benefit that the person it's directed to will actually read your feedback since it would be directly related to work they're doing.