./ahmedhashim

Effective Code Review

Code review catches risks before they hit production and spreads context across the team. Over the years I’ve settled on a small set of practices that keep reviews focused and actionable. Most of them fall on the author, but they’ll make you a better reviewer too.

Problem statement

Open the description with the problem you’re solving and the behavior you expect. The reviewer shouldn’t have to work out your intent from the diff. A sentence on the impact and a link to the issue is usually enough.

If you made a trade-off or worked around a constraint, say so upfront. Otherwise you’ll get feedback on a decision you’ve already made.

Size

SLOC is an imperfect metric, but in my experience the sweet spot for a diff is around 50 to 200 lines, including tests. Past that, reviewers start to lose track of the change. If you can’t explain it in a sentence, it’s probably too big.

Some changes won’t fit in that range, like migrations or large refactors. Split those into a numbered series of stacked PRs so reviewers know what order to read them in, and keep each one to a single intent.

Testability

Reviews tend to stall when the verification steps are fuzzy. Include the exact command along with any seed data or flags, so a reviewer can run it in one shot.

For UI changes, a screenshot beats a paragraph of explanation. For behavior changes, a short checklist does the same job. If there’s a preview environment running the change, link to it in the description.

Rollback plan

Reverts are a normal part of shipping, so assume you’ll need to roll back the same day. That’s a lot easier when the diff only touches the logic it set out to change. Resist the urge to slip in a “small refactor” of the surrounding code.

Put new behavior behind a feature flag or config toggle where you can, and keep irreversible migrations out of diffs that change behavior. If a migration is unavoidable, call it out and describe the rollback path (or admit there isn’t one).

Relevant reviewers

Tag the people who own the code you’re changing and keep the list short. When too many people are tagged, nobody feels responsible for the review. One domain owner is usually enough, plus a generalist if you want a second set of eyes.

If you’re not sure who should sign off, say so in the description and someone can point you to the right person.

Feedback

Spend your comments on correctness and risk. Leave style and formatting to static analysis tools like linters and formatters so they never come up in a review thread.

If a nit is about readability or a team convention the tooling doesn’t cover, raise it once and then write it down so nobody has to raise it again. Label each comment with its priority, using tags like blocker, suggestion, question, and nit, so the author knows what has to change and what’s optional.

You don’t need a new process to adopt any of this. Pick the practice your team trips over most and start with your next PR.