Blog

The Anatomy Of A Perfect Code Review

protect production, not ego
Sterile surgical tray with glowing chips and code snippets under magnifying glass

Code review is not ceremony, not ego arena, and not rubber stamp. It is last human checkpoint before code joins production story.

Perfect code review protects system, author, and users. In that order.

First Question: Are There Tests?

If behavior changes, tests should change. Review without tests becomes reading fiction and guessing ending.

Tests do not need cover every private method. They need prove important behavior and dangerous edge cases.

Review Checklist

Ask simple brutal questions:

  • Does this solve stated problem?
  • Are failure paths handled?
  • Is data validation explicit?
  • Does it scale for expected traffic?
  • Are names clear enough for tired engineer at night?
  • Is logging useful and safe?
  • Is rollback possible?
  • Are tests meaningful, not just coverage decoration?

This checklist catches more bugs than clever nitpicks.

Toxic Versus Constructive

Toxic:

This is stupid. Why would you do it like this?

Constructive:

This path retries non-idempotent transfer calls. That can duplicate side effects if partner times out after processing. Can we persist idempotency key before retry and add test for duplicate callback?

See difference? Same seriousness, no ego slap.

Demand Without Humiliation

Strict review does not mean rude review. Weak review hidden under politeness is also bad.

You can say “I cannot approve this without tests around timeout behavior”. Clear. Professional. No theater.

What Authors Should Do

Make PR small. Explain context. Link ticket. List risks. Show test evidence. Highlight migration or deployment notes.

Do not drop 2,000-line diff and ask “thoughts?” My thought is sadness.

What Reviewers Should Avoid

Do not bikeshed style already enforced by formatter. Do not demand your personal architecture preference without explaining risk. Do not rewrite author code in comments unless teaching.

Focus on correctness, maintainability, security, performance, observability.

Review Is Async Mentoring

A good review teaches how to think. It leaves author stronger. It also leaves written record of design decision.

When junior reads old PR, comments should explain why boundary exists, not just “fixed”.

Approval Means Responsibility

When you approve, you lend your name to change. Treat it like signature on production risk.

No tests, no merge. Unclear failure behavior, no merge. Dangerous retry, no merge.

Protect production. Ego can wait outside.