← Catalog

No. 004 · code-quality

Code Review

Reviews that find real problems without being adversarial

Version 1.0.0 License MIT Format SKILL.md

Code review works best when it focuses on the things that actually matter and skips the rest. Most review friction comes from reviewers treating every line with equal scrutiny — a formatting nit gets the same energy as a logic bug.

What to focus on

Must check:

  • Does the change do what the PR description says?
  • Are there logic errors or edge cases that will blow up in production?
  • Is error handling present and correct?
  • Will this be maintainable by someone who wasn’t in the conversation?

Should check:

  • Is the API surface reasonable (not over-abstracted, not too coupled)?
  • Are there security implications (input validation, auth, secrets)?
  • Do the tests actually test the behavior, not just the happy path?

Skip unless obvious:

  • Style and formatting (that’s what linters and formatters are for)
  • Naming preferences (unless genuinely misleading)
  • “I would have done it differently” (unless the alternative is clearly better)

How to give feedback

Prefix your comments by severity:

  • blocker: — must fix before merge, the code is broken or dangerous
  • suggestion: — consider this, but I won’t block on it
  • nit: — trivial, take it or leave it
  • question: — I don’t understand this, help me see your reasoning

Be specific, not prescriptive:

# Bad
"Change this."

# Good
"This will throw if `user` is null, which happens when the session
expires. Either add a null check or move the auth check earlier."

Ask questions instead of making accusations:

# Bad
"Why did you do it this way?"

# Good
"What's the reason for handling this in the component instead of the
service layer? I want to make sure I'm not missing context."

What not to do

  • Don’t review more than 400 lines at a time — accuracy drops fast
  • Don’t block on anything that a linter could catch
  • Don’t leave comments that are really just opinions stated as facts
  • Don’t review when you’re tired or frustrated — it shows

Self-review checklist for authors

Before requesting review:

  1. Read your own diff end-to-end — you’ll catch half the issues yourself
  2. Write a PR description that answers “why does this exist”
  3. Make sure CI passes — don’t waste reviewer time on red builds
  4. Keep the diff small — if it’s over 500 lines, split it

See references/review-checklist.md for a condensed version.

When it triggers

  • reviewing a pull request
  • giving feedback on someone's code
  • deciding what to flag in a review
  • writing review comments