awesome-cheatsheets/tools/code_review.md

Code Review

Will this code become someone else's problem within three months?

Core Rules

• Walk the five axes in priority order: Correctness → Security → Readability → Performance → Tests.
• Each axis is graded pass / fail / N-A.
• Concrete-fix rule: Every failure must include an exact line reference and a concrete fix.
• Max 10 comments: >10 issues means reject and ask for resubmission, not a 50-item grade sheet.
• No style policing: Formatting, quotes, and spacing belong to linters.

Scope

Diff size	Action
≤ ~400 lines	Review normally
> ~400 lines	Request a split first; review quality collapses on large diffs

The Five Axes

1. Correctness (Blocks merge)

• Edge cases handled: null, empty, zero, negative, boundary/oversized input.
• Concurrency and timing assumptions hold; no race conditions.
• Errors and exceptions handled explicitly, never swallowed.

2. Security (Blocks merge)

• No user input concatenated into SQL, shell commands, or HTML.
• No hard-coded secrets, tokens, or credentials.
• Logs and error messages do not leak sensitive data or PII.

3. Readability

• Descriptive names; functions have a single responsibility.
• Magic numbers are extracted into named constants.
• Flag only incomprehensible logic, not personal style preferences.

4. Performance

• No repeated queries, allocations, or recomputations inside loops.
• No N+1 query patterns across boundaries.
• No data-free speculation ("this might be slow" is invalid feedback).

5. Test Coverage

• New logic and modified paths have automated tests.
• Edge cases and failure modes have dedicated test cases.
• All-green suites that miss critical paths get sent back.

Writing Comments

Fixed Format

[severity][axis] file:line problem → suggested fix

Examples:

[Must fix][Correctness] order.py:87 empty order list raises IndexError → guard before taking [0]
[Should fix][Readability] order.py:92 magic number 86400 → name it SECONDS_PER_DAY

Triage

Label	Axes	Blocks merge?
Must fix	Correctness, Security	Yes
Should fix	Readability, Performance, Tests	No (say so explicitly)

Anti-patterns

Anti-pattern	Failure mode
Drive-by LGTM	Approving before reading the whole diff
Style policing	Nits on quotes while logic defects slip through
Vague feedback	"This looks off" without line number or fix
Speculative perf	Blocking code on unmeasured performance worries
50-item grading	Teaching in review instead of sending back to rewrite

Quick Checklist

- [ ] Scope: diff ≤ ~400 lines (split if larger)
- [ ] Correctness: edge cases handled, exceptions caught, no races
- [ ] Security: input sanitized, no secrets, no sensitive logs
- [ ] Readability: clear names, single responsibility, no magic numbers
- [ ] Performance: no loop queries/N+1, no unverified perf speculation
- [ ] Tests: new paths covered, edge cases tested
- [ ] Fixes: every comment has file:line and a concrete fix (max 10)