Code review is a skill. Here's my approach after reviewing hundreds of PRs.

First Pass: The Big Picture

Before reading any code:

  1. Read the PR description. What problem is this solving? What approach did they take?
  2. Check the size. Under 200 lines? Easy. Over 500? Consider requesting a split.
  3. Look at the file list. Does the scope match the description? Any unexpected changes?

This takes 30 seconds and filters most issues.

What I Actually Check

Correctness

Does the code do what it claims? Not "could this be better" but "does this work?"

  • Edge cases handled?
  • Error paths covered?
  • Assumptions validated?

Readability

Will someone understand this in six months?

  • Clear variable names?
  • Comments where logic is non-obvious?
  • Consistent with codebase style?

Testability

Can this be tested? Is it tested?

  • Tests exist?
  • Tests cover the important cases?
  • Tests are readable?

Security

For anything touching user input or authentication:

  • Input validated?
  • Outputs escaped?
  • Permissions checked?

Performance

Only when it matters:

  • Obvious inefficiencies? (N+1 queries, unnecessary loops)
  • Resource cleanup? (files closed, connections released)

What I Ignore

Style nitpicks. If the linter passes, the style is fine. Don't waste review cycles on brace placement.

Alternative approaches. "I would have done X" isn't useful unless X is meaningfully better. Different isn't wrong.

Future improvements. "You could also add Y" belongs in a follow-up issue, not a PR comment.

How I Give Feedback

Be specific

Bad: "This is confusing" Good: "I don't understand why we check isAdmin here when canEdit already includes admin permissions"

Suggest, don't demand

Bad: "Change this to use a map" Good: "Consider using a map here—O(1) lookup vs O(n) scan"

Distinguish severity

  • Blocking: "This will crash if user is null"
  • Suggestion: "Nit: could rename this to fetchUserById"
  • Question: "Is there a reason we're not using the existing validate helper?"

Praise good work

If something is well-done, say so. "Nice edge case handling here" costs nothing and builds trust.

Common Mistakes I See

Reviewing too slowly. A PR that sits for three days blocks the author. Review within 24 hours or explain the delay.

Reviewing too much at once. Better to do three passes over a day than one exhausted pass. Fresh eyes catch more.

Focusing on the wrong things. Spending 20 minutes debating naming while missing a null pointer.

Being adversarial. Review is collaboration, not competition. You're both trying to ship good code.

What Makes a Good Reviewer

  1. Fast turnaround. Unreviewed PRs rot. Priority is responsiveness.
  2. Calibrated feedback. Know when to block vs suggest vs let it go.
  3. Teaching mindset. Explain the why, not just the what.
  4. Humility. You might be wrong. Ask questions before asserting.

My Checklist

Before approving:

  • I understand what this code does
  • The approach makes sense for the problem
  • Edge cases and errors are handled
  • Tests exist and cover important paths
  • No obvious security issues
  • No obvious performance issues
  • I'd be comfortable maintaining this code

If all boxes are checked, approve. If any aren't, comment specifically on what's needed.

Code review isn't about perfection. It's about catching problems and sharing knowledge. Keep that goal in mind.

React to this post: