Effective Code Review
Reviewing code is one of the most important parts of software development. It catches risks before they hit production, spreads context across the team, and creates opportunities to learn from each other.
Over the years, I’ve settled on a small set of practices that keep reviews focused, actionable, and respectful. You can apply them whether you’re writing the change or reviewing it.
Problem statement
Use the description to state the problem and the intended behavior, not just the implementation. Don’t make the reviewer infer intent from the diff. A sentence on the impact and a link to the issue is usually enough.
If there’s a trade-off or constraint, say it upfront so feedback stays on the right axis.
Size
While SLOC is an imperfect metric, the sweet spot for a diff is usually around 50-200 lines, including tests. Once a diff gets larger than that, reviewers start to lose track of the change. If you can’t explain it in a sentence, it’s probably too big.
If a change can’t realistically fit in that range (for example, migrations or large refactors), split it into stacked PRs with a clear review order. Use a numbered series so reviewers know the intended path. One intent per diff keeps feedback quality high.
Testability
Reviews stall when verification steps are fuzzy. Include the exact command, flags, and any seed data so a reviewer can run it in one shot. If you can’t run it quickly, you shouldn’t trust it.
For UI or behavior changes, add a quick checklist or an image of the change. Pictures are worth a thousand words, and a screenshot beats a paragraph of explanation. If there’s a preview environment with a live version of the change, link to it in the description.
Rollback plan
Assume you might need to roll it back the same day. Keep changes isolated and focused on the logic being changed. Don’t take this as an opportunity for a “small refactor” to the surrounding code.
Prefer feature flags or config toggles, and avoid bundling irreversible migrations with behavior changes. Reverts are part of shipping, not a failure.
If a migration is unavoidable, call it out and describe the rollback path (or the lack of one).
Relevant reviewers
Tag the people who own the surface area you’re changing and keep the list tight. Too many reviewers means no one feels responsible. Prefer one domain owner and one generalist if needed.
If you’re unsure, ask in the description and add a note about who should sign off.
Feedback
Focus comments on correctness, risk, and maintainability. Style and formatting should be handled by static analysis tooling, not review threads.
If a “nit” affects readability or a team convention not covered by tooling, call it out once and codify it. Make comment priority explicit with labels like blocker, suggestion, question, and nit so authors know what needs to change and what’s optional.