[hinkley]

On the first team where we did code reviews, I found in RCAs for the bad bugs that snuck in that there was a dearth of comments on the CR that introduced it.

But usually the severity was more correlated with a lack of comments on the tests. Giant holes in test cases meant giant bugs. So don’t call a PR ready to land until you’ve gotten a few substantive critiques. Because the absence of evidence is not evidence of absence.

For the bugs I introduced (I am highly bug-averse) it was either zero comments on tests, or bugs introduced by the CR process - changes I was coerced into making that I felt were wrong. And I can’t say which sort of subconscious resistance was at work there. Self sabotage for making changes I don’t want to, or my sixth sense for robust code telling me the suggestion is an antipattern. Probably both.

What I learned from that last, which I confirmed in subsequent years, was that as a team you should only tolerate major pushback on a CR/PR at the beginning of the review process. Anyone who jumps in late with Needs Work demands, especially after a round of feedback changes has already landed, has lost their right to participate in the review. Because as a PR drags on, everyone gets tired of looking at it and has a less critical eye for spotting bugs that have been introduced by committee. It quickly becomes better odds that the original bugs the early reviewers did not catch are less dangerous than the ones that will be introduced by work-hardening the PR.

It’s the same mechanic that makes pushing code or a deployment after 4pm a bad idea. Confirmation bias is greatly amplified by a desire to be somewhere else.