PR Review Checklist

Walk this list before you click “Approve”. If anything is unchecked, leave a comment instead.

Scope

• PR does one thing — title describes that thing in one line
• No drive-by refactors mixed with the change
• Diff fits in your head (< 400 lines, or split)

Correctness

• Acceptance criteria from the linked ticket are met
• Edge cases handled: empty input, max input, concurrent calls, network failure
• Off-by-one and timezone bugs ruled out
• Error paths return useful errors, not silent failures

Tests

• New code has tests — at least one per acceptance criterion
• Tests assert behaviour, not implementation
• Tests would actually fail if the code regressed
• No skip, xit, it.only, or commented-out assertions

Readability

• Names describe intent, not type or position
• Comments explain WHY, not WHAT
• No magic numbers, no copy-pasted blocks
• Public APIs documented

Security

• User input validated at boundary
• No secrets / tokens / keys in code or tests
• Authorization checked, not just authentication
• No new SQL injection / XSS / SSRF surface

Performance

• N+1 queries ruled out (count queries in tests)
• No unbounded loops or unbounded result sets
• Indexes added for new query patterns
• Heavy work backgrounded, not in request cycle

Operations

• Migrations are reversible (or explicitly forward-only)
• Feature flag added for risky changes
• Logs / metrics added for new flows
• Runbook / docs updated if behaviour changed

Approve / request changes

• Approve if every box is checked
• Request changes if any blocker is unchecked
• Comment if you have suggestions but no blockers

Definition of Done glossary: https://sprintflint.com/glossary/definition-of-done