Yes, but they are unconvincing. Pair programming is only acceptable to some developers (others it drives insane). It also induces tunnel vision because now both developers are too close to the code to see issues that an outside observer will pick up quickly. They're also intimately familiar with the structure and algorithms, and so they won't see things that someone fresh would have trouble grasping (meaning that they need to either simplify, name with intent, or comment appropriately). I actually haven't noticed too much of a difference in issues picked up on pair programmed vs solo programmed PRs; they tend to match the quality of the stronger developer.
I've done periodic code reviews at three separate companies. Unless I've been very unlucky, it generally goes like this:
* Everyone gets pulled away from their work for an hour and goes into a room.
* Someone has picked some code to review (either they've been forced into it, or they have something cool they want to show off). Mostly they're just terrified at the prospect of being humiliated in front of everyone.
* Everyone pretends to be interested, but they're really just waiting for it all to end as the CO2 levels slowly climb.
* A few minor details are talked about, and then someone asks why approach Y wasn't used. Relieved that there's finally a discussion point, everyone talks about this architectural idea for the rest of the hour.
* Due to time constraints, 90% of the code is never reviewed.
I spend about an hour a day on average reviewing PRs, and that's with a team of 5 engineers. If we reviewed all code in team meetings, that would multiply review time by 5x because now everyone is reviewing all code in series rather than in parallel.
Then there's this:
> Pipeline approvals: If your team uses a Continuous Delivery pipeline to deliver changes to production, you can include a stage that requires someone to authorize the change to progress. This is conceptually similar to a pull request in that it is a gate in the delivery process, but you place the gate after code integration and automated tests. Doing this means that a human only spends time reviewing code that has already been proven technically correct.
Except it hasn't been proven technically correct. It's just code that has been (maybe) pair programmed, and (maybe) discussed if it happened to be part of the 10% of the code looked at and discussed for architecture in the review meeting.
I've done periodic code reviews at three separate companies. Unless I've been very unlucky, it generally goes like this:
* Everyone gets pulled away from their work for an hour and goes into a room.
* Someone has picked some code to review (either they've been forced into it, or they have something cool they want to show off). Mostly they're just terrified at the prospect of being humiliated in front of everyone.
* Everyone pretends to be interested, but they're really just waiting for it all to end as the CO2 levels slowly climb.
* A few minor details are talked about, and then someone asks why approach Y wasn't used. Relieved that there's finally a discussion point, everyone talks about this architectural idea for the rest of the hour.
* Due to time constraints, 90% of the code is never reviewed.
I spend about an hour a day on average reviewing PRs, and that's with a team of 5 engineers. If we reviewed all code in team meetings, that would multiply review time by 5x because now everyone is reviewing all code in series rather than in parallel.
Then there's this:
> Pipeline approvals: If your team uses a Continuous Delivery pipeline to deliver changes to production, you can include a stage that requires someone to authorize the change to progress. This is conceptually similar to a pull request in that it is a gate in the delivery process, but you place the gate after code integration and automated tests. Doing this means that a human only spends time reviewing code that has already been proven technically correct.
Except it hasn't been proven technically correct. It's just code that has been (maybe) pair programmed, and (maybe) discussed if it happened to be part of the 10% of the code looked at and discussed for architecture in the review meeting.