Pull Request Review Checklist

What this template is

This is a reviewer’s checklist for pull requests. It is written for teams whose reviews have drifted into style nitpicks and rubber stamps, and for new reviewers who want a repeatable procedure instead of “read it and see what jumps out.”

A PR review is not a code audit. An audit examines the whole system at rest; a review examines one change in motion. The two catch different things, and the difference matters (we compare them in code audit vs code review). This checklist keeps the review focused on what a diff-level check can actually catch: correctness of the change, new attack surface, missing tests, and readability.

How to use it: paste the markdown into your PR template (.github/PULL_REQUEST_TEMPLATE.md) or your team wiki. A reviewer should get through it in 10 to 20 minutes on a normal-sized PR. If a PR is too large to review against this list in 30 minutes, the correct review comment is “split this.”

1. Before reading a line of code

• The PR description says what changed and why; if you cannot tell, ask before reviewing
• The diff is under ~400 changed lines; larger PRs get exponentially worse review quality, so request a split
• CI is green; do not spend human attention on code the machine already rejected
• The change matches the ticket; scope creep gets flagged, not silently approved

2. Correctness

• Read the change as if you had to explain it to the author; every branch and early return makes sense
• Edge cases are handled: empty input, null, zero, negative numbers, unicode, very large input
• Off-by-one candidates checked: loop bounds, slicing, pagination cursors, date ranges
• Concurrent access considered where relevant: what happens if this runs twice at the same time?
• Failure paths are real: what happens when the network call fails, the row is missing, the queue is full?

3. Security

• No secrets in the diff: keys, tokens, passwords, internal URLs (run gitleaks protect --staged locally; setup in the secrets scanning tutorial)
• New endpoints check both authentication and authorization server-side
• Any new query is parameterized; any rendered user input is escaped
• New dependencies are justified, actively maintained, and free of known critical CVEs
• Nothing sensitive is added to logs or error messages

4. Tests

• The change includes tests, or the description says why not and the reviewer agrees
• Tests cover the behavior that changed, not just lines: at least one test would fail if the fix were reverted
• Bug fixes include a regression test that reproduces the original bug
• Tests assert outcomes, not implementation details; renaming a private method should not break them

5. Readability and design

• Names say what things are; a reader who knows the domain but not this PR can follow it
• No dead code, commented-out blocks, or leftover debug statements
• The change lives in the right layer: business logic is not in the controller or the template
• Duplication introduced by the diff is deliberate, not accidental
• Public-facing behavior changes come with doc or changelog updates

6. Performance and data

• New queries in loops are flagged (the classic N+1); fetch in bulk instead
• New database columns used in lookups have indexes, and migrations are backward compatible with the running version
• Unbounded work is bounded: pagination on new list endpoints, limits on batch sizes
• Large payloads, uploads, or exports stream instead of buffering everything in memory

How to act on the results

Not every unchecked box blocks a merge. Use three response levels and label your comments accordingly. Blocking: correctness bugs, security issues, missing tests on risky paths. Non-blocking but expected: naming, structure, minor duplication; the author fixes them in this PR or a fast follow-up. Optional: preferences, marked as such. Reviews go bad when all three get flattened into an undifferentiated wall of comments, because authors then treat everything as optional.

Track what slips through. When a production bug traces back to a merged PR, ask which checklist item would have caught it; if none, add one. And know the limits of the tool: diff review cannot see cross-cutting problems like systemic missing authorization or dependency rot across the whole tree. Those need a periodic whole-codebase pass; the vulnerability classes that reviews consistently miss are cataloged in top security vulnerabilities code audits catch, and periodic checks like a dependency audit cover what no single PR review can.

Want us to run it for you (free)

PR review keeps new code healthy; it does nothing for the code that was merged before the checklist existed. If you want to know what is already in your codebase (the accumulated misses of every past review), submit your repository and a Webisoft engineer will audit the whole tree manually and send you a prioritized report, free: get a free code audit.