Code Review Checklist
A code review checklist for pull requests: intent, correctness, tests, security, data changes and readability. Use it for risky PRs and keep a review record.
- Typical schedule per pull request
- The checklist 6 sections, 19 items
- Variables
{{repository}}{{pr_number}} - Start from CI
Use this checklist when you review a pull request that deserves more than a quick glance: changes to authentication, payments, data deletion, permissions, public APIs or database schemas. For small, routine changes your normal review is enough. The checklist helps reviewers look at the things that are easy to miss when you only read the diff from top to bottom.
Who owns it
The reviewer owns the run. The author prepares the pull request so it can be reviewed: a clear description, linked ticket, and tests. If a second reviewer with special knowledge is needed, such as for security or the database, assign those items to them. In teams that need evidence of peer review for an audit, the completed run is that evidence.
Common pitfalls
- Reviewing style instead of behaviour. Formatting and naming should be handled by a linter and a formatter. Spend review time on logic, edge cases and failure modes.
- Pull requests that are too large. Nobody reviews a large diff well. Ask the author to split it instead of approving it on trust.
- Only reading what changed. Bugs often hide in the code that was not changed but now gets called differently. Look at the callers.
- Approving without running anything. For UI or behaviour changes, check out the branch or open the preview environment and try it.
- Vague comments. Mark which comments block the merge and which are suggestions, so the author knows what must change.
The checklist: Review {{repository}} PR {{pr_number}}
Context
-
PR
{{pr_number}}has a description that explains what changes and why, with a linked ticket -
The change is small enough to review properly, or the author has been asked to split it
-
CI checks, linters and tests pass on the latest commit
Correctness
-
The code does what the ticket asks, including the edge cases named in it
-
Error handling covers failures of external calls, empty input and timeouts
-
Callers of changed functions still behave correctly
-
Concurrency, retries and idempotency are handled where jobs or requests can run twice optional
Tests
-
New behaviour is covered by tests that would fail without the change
-
Tests check results, not only that the code runs without errors
-
The change was tried by hand in a branch build or preview environment optional
Security and data
-
Input from users or other systems is validated and output is encoded where needed
-
Authorisation is checked for every new endpoint or action
Confirm that a user cannot reach another user's or another organisation's data by changing an ID.
-
No secrets, tokens or personal data are committed or written to logs
-
New dependencies are needed, maintained and have an acceptable licence optional
-
Database migrations are backwards compatible and safe on large tables optional
Maintainability
-
Names and structure make the code understandable without the PR description
-
Docs, comments or the changelog are updated where behaviour changes
-
Review comments are marked as blocking or as suggestions
Decision
-
Review outcome is recorded: approved, changes requested or needs a second reviewer
Checklist as Markdown
Start this checklist from your pipeline
When a pull request gets the label high-risk, POST /api/v1/runs with template "Review {{repository}} PR {{pr_number}}" and the PR number.
curl -X POST https://runbook.metastic.ch/api/v1/runs \
-H "Authorization: Bearer $RUNBOOK_TOKEN" \
-H "Content-Type: application/json" \
-d '{"template": "Review {{repository}} PR {{pr_number}}", "variables": {"repository": "…", "pr_number": "…"}}'Frequently asked questions
What should a code reviewer check first?
Start with intent: read the description and ticket, then decide whether the approach makes sense before you look at details. Next check correctness and tests, then security and data handling. Style comes last, and most of it should be automated.
How big should a pull request be for a good review?
Small enough that a reviewer can understand it in one sitting. There is no fixed line count, but if you cannot hold the whole change in your head, it is too big. Split it into a refactoring step and a behaviour change, or into backend and frontend parts.
Should we use a checklist for every pull request?
Usually not. A checklist for every small change becomes a box-ticking habit. Use it for high-risk changes, which you can mark with a label in your Git host, and start the run from CI when that label is added.
Updated