Code Review Checklist

Open the linked Jira/Linear ticket and confirm the PR scope matches what was agreed during refinement. PRs that drift from the ticket — added refactors, opportunistic cleanups, scope creep — are the most common source of review friction. Flag scope mismatches in a top-level review comment before going line-by-line.

Don't review only in the GitHub/GitLab diff view for non-trivial PRs. Pull the branch, run it, exercise the changed code path. The diff hides cross-file context; running it locally surfaces issues like broken local dev setup, missing env vars, or unrunnable tests.

Don't review a red build. If a check is failing, ask the author to fix or explicitly flag it as a known flake before continuing. "Just rerun it" becomes a habit that masks real regressions.

Review quality drops sharply on PRs over ~400 lines of net diff. Categorize before you start reading; if it's oversized, request a split rather than skim-approving with "LGTM".

If your toolchain has ESLint, Prettier, RuboCop, gofmt, ruff, or equivalent — these should already run in CI. Spot-check that the configured rules actually ran on the changed files (no skipped files via .eslintignore or //eslint-disable). Style nits are for the linter, not the reviewer.

Flag ambiguous variable names (data, tmp, handler2), unused imports, commented-out code blocks, and stray console.log / puts / print debug statements. These are cheap to fix now and expensive to clean up later.

For each new branch in the code, look for a corresponding test case. Coverage percentage alone is misleading — a 90% line-coverage PR can still skip the actual conditional logic. Read the test names and assertions, not just the coverage delta.

If the PR changes an HTTP endpoint, gRPC method, or message contract, an integration test should exercise the request/response shape end-to-end. Unit tests with mocked clients miss serialization bugs and middleware ordering issues.

Does this change belong in this service? PRs that introduce cross-service coupling (a billing concept leaking into the auth service, a UI module reaching into a backend repo) are architecture smells. If unsure, loop in the service owner via CODEOWNERS or a tech lead.

Grep the codebase for the new utility's name, signature, or core logic before approving. Reinvented date parsers, retry helpers, and HTTP client wrappers proliferate when reviewers don't ask. If a similar helper exists, push back unless the author has a specific reason not to use it.

Watch for N+1 queries on list endpoints, unbounded loops over user input, in-memory aggregations that will OOM at 10x scale, and god-classes that violate single responsibility. "It works for our current data size" is not a passing answer.

Look for files under db/migrate, migrations/, Alembic, Flyway, golang-migrate, or schema.rb/schema.prisma changes. A schema change deserves its own review pass — these are the highest-blast-radius changes in the diff.

API keys, database URLs, JWT secrets, AWS access keys, internal hostnames in test fixtures. GitHub secret scanning + gitleaks/trufflehog in pre-commit catch most cases, but a human pass on the diff is the last line of defense. If a secret lands on main, rotation alone doesn't fix it — the value lives in git history forever without filter-repo or BFG.

Every new endpoint needs an explicit authorization check, not just authentication. Confirm the user can only access resources they own (IDOR protection), and that input is validated before hitting the database (SQL injection, command injection, OWASP Top 10). "The frontend prevents this" is not an authorization model.

Check package.json / Gemfile / go.mod / pyproject.toml diff for new entries. Each new dep adds maintenance surface, license obligations, and supply-chain risk. Worth scrutiny even if it's a one-line install.

Use the platform's review action — Approve, Request changes, or Comment. Don't approve with unresolved blocking comments; either resolve them or use "Request changes" so the PR can't be merged. Reviewer notes capture context (what you focused on, what you skipped) for the audit trail and for the next reviewer if approval is delegated.

Confirm squash vs. rebase vs. merge-commit matches the repo's branch protection rules. Confirm CODEOWNERS approvals are satisfied. If the commit message will become the squash message, make sure it's not "fix typo (#1234)" — clean it up before merge so the main-branch history stays readable.