Code Review Checklist for Focused, Evidence-Based Reviews

A useful code review answers a small set of consequential questions: does this change solve the stated problem, what could it break, and what evidence supports merging it? Reviewing every line with equal intensity wastes attention. A checklist provides coverage, but the reviewer still needs judgment about the paths that carry the greatest risk.

Establish context and choose the review depth

Read the issue, acceptance criteria, and pull-request description before opening the diff. State the expected behavior in your own words. If the goal is unclear, ask the author to clarify it before discussing implementation details. Intent before syntax prevents a beautifully reviewed solution to the wrong problem.

For structured payload changes, use the JSON formatter with synthetic or redacted before-and-after examples. Compare keys, nesting, nullability, and omitted values without the distraction of compressed data. This supports visual inspection; it does not prove schema compatibility or validate authorization rules. Keep private customer data out of shared tools.

Classify the change by consequences rather than line count. A one-line permission change can deserve more scrutiny than a large mechanical rename. Identify affected users, persistence, public interfaces, external services, and failure recovery. Request a separate patch when unrelated refactoring obscures the actual behavior change.

Review correctness through complete execution paths

Choose one successful scenario and trace it from input to output. Then choose a failure scenario and follow it with equal care. Check branches, early returns, default values, and state transitions. Boundary cases often reveal the real defect: zero items, missing input, duplicate submission, expired state, or a response arriving after the user navigated away.

Use a concrete checklist rather than asking whether the code “looks right”:

• Are input assumptions enforced where untrusted data enters?
• Do callers handle the function's actual return and error behavior?
• Can retries repeat a side effect that should happen only once?
• Are resources and subscriptions released after failure or cancellation?
• Does the change preserve compatibility for existing consumers?

Consider this superficially tidy example:

async function savePreferences(input: unknown) {
const preferences = parsePreferences(input);
const response = await fetch("/api/preferences", {
  method: "PUT",
  headers: { "Content-Type": "application/json" },
  body: JSON.stringify(preferences),
});
if (!response.ok) {
  throw new Error("Saving preferences failed");
}
}

The reviewer still needs to inspect the parser, server-side authorization, caller error handling, and behavior under concurrent saves. Checking response.ok matters because HTTP error statuses do not normally reject the fetch promise; consult the MDN Fetch guide for exact behavior. A short function is not necessarily a small review surface.

Inspect security and data handling separately

Make trust boundaries an explicit pass instead of hoping security problems stand out during general reading. Identify which values originate from users, remote systems, stored records, or environment configuration. Follow those values into database operations, HTML output, filesystem paths, logs, and network destinations.

Check that permissions are enforced on the server for the requested resource, not merely hidden in the interface. Verify that validation does not silently convert a dangerous value into an apparently acceptable one. Avoid logging tokens, personal information, or complete request bodies just because a debugging helper makes it easy.

Use the OWASP Code Review Guide as a source of deeper security questions when the change touches sensitive behavior. A checklist is not a replacement for specialist review of authentication, cryptography, payment handling, or significant access-control changes. Escalation is a sign of correct risk assessment, not reviewer failure.

Inspect dependency and configuration changes too. A new package, broadened permission, or modified installation script can matter more than the application code. Confirm why the dependency is needed and whether existing project utilities already cover the requirement.

Evaluate tests as evidence, not decoration

Start with the defect or requirement and ask which test would fail if the implementation were wrong. Regression evidence is stronger than a screenshot of a green suite unrelated to the changed behavior. For a bug fix, confirm that the new test fails against the prior implementation when practical.

Look for meaningful assertions about observable results. A test that only checks a mock was called may miss an incorrect payload or a swallowed error. Conversely, tests tightly coupled to private helper order can block harmless refactoring without protecting users. Favor the public behavior at the narrowest useful testing level.

Read the failure cases: rejected requests, invalid input, partial results, cancellation, and repeated actions. For interface changes, include a manual keyboard pass and inspect focus after errors. The accessible interface guide offers concrete prompts beyond checking whether an automated scan is green.

Record which commands ran and what was not exercised. Distinguish a flaky test, an unavailable external service, and a genuine regression. Do not normalize unexplained failures just because they occurred before this pull request; document ownership and determine whether they undermine the evidence needed for this change.

Leave actionable feedback and close the loop

Separate merge blockers from optional improvements. A blocker should name the violated requirement, explain the consequence, and propose a way to demonstrate a fix. A suggestion should be clearly optional. This reduces guesswork and avoids turning personal preferences into unofficial policy.

For example, replace “this is unsafe” with “the server accepts another user's record identifier; please add an ownership check and a test showing a forbidden response.” Replace “clean this up” with a specific duplication concern and the existing helper that addresses it. Ask questions when context is missing rather than asserting motives or competence.

Automate formatting and routine lint decisions so human attention goes to behavior. The Google engineering review practices provide a useful model for balancing code health with timely progress. Keep team-specific conventions documented alongside the repository, with supporting material in your developer-tool workflow resources.

After revisions, review the new diff and the surrounding behavior, not only the comment resolution markers. Approval is a decision about the current patch. If a major redesign arrives after approval, take another pass. Capture any accepted follow-up as a tracked task with an owner rather than an indefinite promise in a resolved conversation.

Key takeaways

• Start with intent, consequences, and the riskiest execution paths.
• Review correctness, trust boundaries, and test evidence in separate passes.
• Prefer observable behavior over implementation-shaped assertions.
• Label blockers and suggestions so authors can act without guessing.
• Recheck revisions and make remaining risk explicit before approval.