Confessions of a programmer: I hate code review (2010)

Confessions of a programmer: I hate code review (2010)
blog.nelhage.com

"Did you know you can replace that entire function with a regex?"

This is what I see day in day out: we have a lot of tooling for the reviewing process but don't seem to have any or very little tooling for the design/discussion phase _before_ someone starts implementing...

1. Both reviewer and reviewee are focusing on getting the best outcome possible, in good faith and with generosity.

2. The reviewer concedes that there can be equally valid approaches to a given problem or taste wrt. aesthetics. They do not try to gratuitously force their style upon the reviewee. The reciprocal must hold as well.

3.The reviewer makes practical suggestions and ideally accompany their comments with snippets of code. They involve themselves into being part of the solution.

4. The reviewer focuses on the substance of the pull request. That means putting in the time to understand the real logic, putting themselves in the shoes of the reviewee (with their help), and trying to challenge it so that the limitations of the approach are known and documented.

5. The reviewee makes an effort at slicing their request into readable and compact sub-PRs if needed. Similarly, the reviewer makes a best-effort attempt at starting their review within a reasonable time.

Those are central aspects of a productive engineering culture anyway. You only need to find someone else that shares those "values" (i.e focusing on the best outcome possible as a sort of devotion to Engineering - so to speak) to grow this attitude within your company.

Shipping bugs.

The author of this piece admits that they have to change the code they write in response to comments. This means the code review is flagging areas of improvement. Surely the resulting code is better than the original code (otherwise I'd expect the author to push back on the comments instead of implementing bad suggestion).

Similarly, when acting as the code reviewer, the author is giving feedback to others (rather than just rubber-stamping the PR), and presumably that feedback is implemented.

So code review seems to be doing its job here. It's producing better code. Ultimately I think the problem is revealed at the end, where the author says they feel like code review is optional. If you think it's optional, then the delays inherent in review are going to feel like artificial slowdowns.

The better way to think about it is that code review simply isn't optional. Yes it can be a pain, and there's a lot of friction in the process, and our tooling kinda sucks for it. But it's a necessary step. I can't even begin to count how many bugs my team has avoided shipping due to code review. And I always get really annoyed when I run across a bug that should have been caught in code review but wasn't for whatever reason (perhaps a more junior coder did the review and therefore missed stuff that a more senior coder would have caught). Code review stops bugs before they happen. And the best bug is the one that was never shipped to customers.