A good code review catches real problems, improves the codebase, and transfers knowledge — without being adversarial or slowing the team down. Use this checklist to be thorough without being exhaustive.
Before You Start
- Read the PR description — understand what it's trying to accomplish and why
- Check if there's a linked ticket or specification to reference
- Pull the branch and run it locally for non-trivial changes
Correctness
- Does the code actually do what the PR description says it does?
- Are there edge cases that aren't handled? (null values, empty arrays, large inputs, concurrent requests)
- Do the tests cover the happy path AND failure paths?
- If tests are missing, are the untested parts trivial or risky?
- Does the logic handle race conditions if the code is async?
- Are error messages clear and actionable?
Security
- Is user input validated and sanitized before use?
- Are database queries parameterized (no string concatenation)?
- Is any sensitive data (passwords, tokens, PII) logged or exposed?
- Are new API endpoints protected with authentication and authorization?
- Are file uploads validated for type and size?
- Are any new secrets hardcoded? (Should be in env vars)
Performance
- Are there any N+1 queries? (Fetching data in a loop that could be a single query)
- Are expensive operations cached where appropriate?
- Will this work at 10x the current data volume?
- Are database indexes needed for any new query patterns?
- Are large payloads paginated?
Code Quality
- Is the code readable — would a new team member understand it in 6 months?
- Are function and variable names descriptive and accurate?
- Is there duplicated code that should be extracted to a shared function?
- Are there any TODO comments that should be tickets instead?
- Is the code consistent with the patterns used elsewhere in the codebase?
- Is dead code removed, not commented out?
Error Handling
- Are errors handled explicitly — no swallowed exceptions (
catch)? - Do errors bubble up correctly or are they handled at the right level?
- Do external calls (APIs, DB) have timeouts?
- Is there appropriate retry logic for transient failures?
Documentation
- Is complex business logic explained with comments?
- Are public APIs and function signatures documented?
- Is the README or wiki updated if behavior has changed?
- Are breaking changes documented in a changelog?
How to Give Feedback
- Prefix comments by type: nit: (minor, optional), question: (seeking understanding), blocker: (must fix before merge)
- Suggest, don't demand: "What do you think about extracting this to a helper?" not "Extract this."
- Explain the why: "This could cause a SQL injection — use parameterized queries instead"
- Leave positive comments too — acknowledge good solutions
- Be responsive — don't leave PRs waiting more than 1 business day