Code Review Checklist
A code review checklist provides a systematic framework for evaluating code changes across every quality dimension that matters. Without a checklist, reviewers tend to focus on the areas they know best and overlook the rest. A security-focused reviewer might catch every injection vulnerability but miss accessibility issues. A performance-minded reviewer might optimize every query but overlook missing error handling. A checklist ensures comprehensive coverage regardless of the individual reviewer's strengths.
This is not a checklist to follow mechanically on every pull request. Not every item applies to every change. A PR that modifies a database migration script does not need an accessibility review, and a PR that fixes a CSS bug does not need a security audit. The value of the checklist is as a reference that ensures you consider each dimension when it is relevant, not as a gate that every PR must pass through in its entirety.
Use this checklist as a starting point and customize it for your team and project. Add items specific to your technology stack, remove items that are enforced by automated tools, and adjust the categories to match your team's priorities.
Correctness
Correctness is the most fundamental quality dimension: does the code do what it claims to do? A feature that does not work correctly is worse than no feature at all, because it creates false confidence and makes debugging harder when things eventually fail.
- Does it do what the PR description says? Read the description, then read the code. Do they match? Are there changes in the diff that are not mentioned in the description, or claims in the description that are not reflected in the code?
- Are edge cases handled? What happens with empty inputs, null values, extremely large values, negative numbers, Unicode characters, or concurrent access? Think about the boundary conditions that the author might not have considered.
- Is error handling complete? Does the code handle failure cases gracefully? Are exceptions caught at the right level? Are error messages informative without leaking sensitive information? Does the code clean up resources (database connections, file handles, locks) when an error occurs?
- Are assumptions documented? If the code assumes a certain input format, a certain state of the database, or a certain order of operations, are those assumptions documented in comments or enforced with assertions?
- Does it handle the unhappy path? It is easy to verify that code works when everything goes right. The real test is what happens when things go wrong: network timeouts, disk full, permission denied, invalid input, downstream service unavailable.
- Are there off-by-one errors? Check loop boundaries, array indices, pagination limits, and date range calculations. Off-by-one errors are among the most common bugs in software and are easy to miss during development.
Security
Security issues are the most consequential bugs you can ship. A single SQL injection vulnerability can expose your entire database. A single XSS vulnerability can steal user sessions. Security review should be thorough and deliberate, even for seemingly innocuous changes.
- Is user input validated and sanitized? Never trust input from users, APIs, or any external source. Validate that input matches expected formats, lengths, and ranges before processing it. Sanitize input before displaying it or including it in queries.
- Is the code resistant to SQL injection? All database queries should use parameterized statements or a query builder that handles escaping. Never concatenate user input directly into SQL strings.
- Is the code resistant to XSS? All user-generated content displayed in HTML must be properly escaped. Verify that template engines are configured to escape output by default. Check for cases where raw HTML rendering is used and ensure the content is trusted.
- Are authentication and authorization checks in place? Does every endpoint verify that the user is authenticated? Does it verify that the authenticated user has permission to perform the requested action? Are there endpoints that accidentally bypass these checks?
- Are secrets properly managed? Check for hardcoded API keys, passwords, tokens, or connection strings. These should come from environment variables, a secrets manager, or a configuration system — never from source code.
- Is sensitive data protected? Are passwords hashed (not encrypted or stored in plain text)? Is personal data encrypted at rest and in transit? Are credit card numbers, social security numbers, or other sensitive data masked in logs and error messages?
- Are dependencies safe? If the PR adds new dependencies, check whether they are actively maintained, have known vulnerabilities, and are from trusted sources. Run
npm auditor equivalent for your package manager.
Accessibility
Accessibility ensures that your application works for all users, including those who use screen readers, keyboard navigation, voice control, or other assistive technologies. Accessibility issues affect a significant portion of users and, in many jurisdictions, are a legal requirement.
- Is semantic HTML used? Are headings used in the correct hierarchy (h1, h2, h3)? Are lists used for lists? Are buttons used for actions and links used for navigation? Semantic HTML provides structure that assistive technologies rely on.
- Are ARIA attributes correct? If custom components require ARIA attributes, are they applied correctly? Common mistakes include missing
aria-labelon icon buttons, missingroleattributes on custom widgets, and incorrectaria-expandedoraria-selectedstates. - Does keyboard navigation work? Can all interactive elements be reached and activated using only the keyboard? Is the tab order logical? Are there keyboard traps where focus gets stuck? Do modal dialogs properly manage focus?
- Is color contrast sufficient? Text should have at least a 4.5:1 contrast ratio against its background (3:1 for large text). Do not use color alone to convey information — always provide an additional indicator like an icon, pattern, or text label.
- Do images have alt text? All informational images should have descriptive alt text. Decorative images should have empty alt attributes (
alt="") so screen readers skip them. Complex images (charts, diagrams) should have detailed descriptions. - Are form fields labeled? Every form input must have an associated label. Placeholder text is not a substitute for a label because it disappears when the user starts typing.
Performance
Performance problems are insidious because they often do not cause visible errors. The application works, but it works slowly. Users leave, SEO rankings drop, and server costs increase. Review code changes with performance in mind, especially in hot paths and data-heavy operations.
- Are there N+1 query patterns? The most common database performance issue is making one query to fetch a list of items, then making a separate query for each item to fetch related data. Look for database calls inside loops and verify that eager loading or batch queries are used instead.
- Are there unnecessary re-renders? In frontend frameworks like React, unnecessary re-renders waste CPU cycles and make the UI feel sluggish. Check for missing memoization, incorrect dependency arrays in hooks, and state updates that trigger re-renders of components that have not actually changed.
- Is bundle size considered? Does the PR add new dependencies that significantly increase the bundle size? Are large libraries imported when only a small utility function is needed? Can new code be lazy-loaded or code-split to avoid impacting initial page load?
- Is caching used appropriately? For expensive computations or frequently accessed data that changes infrequently, caching can dramatically improve performance. Check whether the PR introduces opportunities for caching or whether it invalidates existing caches correctly.
- Are database queries optimized? Check for missing indexes on columns used in WHERE clauses, JOINs, or ORDER BY statements. Verify that queries do not select more data than needed (avoid
SELECT *when only a few columns are required). - Are large datasets paginated? Any endpoint or query that returns a list of items should be paginated. Fetching thousands of records when the user only sees 20 at a time wastes bandwidth, memory, and processing time.
Testing
Tests are the long-term safety net that ensures the code continues to work correctly as the codebase evolves. A PR without tests is a PR that will eventually break without anyone noticing until a user reports the bug.
- Are tests present for new functionality? New features, endpoints, components, and utility functions should have corresponding tests. If the PR adds behavior, it should add tests for that behavior.
- Do the tests test the right things? Tests should verify behavior, not implementation details. A test that asserts a specific method was called is brittle and breaks when the implementation changes. A test that verifies the expected output for a given input is resilient and catches real regressions.
- Are edge cases tested? Beyond the happy path, do tests cover error cases, boundary conditions, and unusual inputs? Does the test suite verify that the code fails gracefully when things go wrong?
- Are the tests readable? Test code is documentation. Can a developer unfamiliar with the code understand what each test verifies and why? Are test names descriptive? Is the arrange-act-assert structure clear?
- Do tests run independently? Tests should not depend on each other or on shared mutable state. Each test should set up its own preconditions and clean up after itself. Tests that depend on execution order are fragile and produce intermittent failures that waste debugging time.
Documentation
Code changes often require corresponding documentation changes. Missing documentation creates a gap between what the code does and what users and developers understand it to do.
- Is the changelog updated? If your project maintains a changelog, has this PR's change been recorded? Is the entry in the correct section (added, changed, fixed, removed)?
- Are API changes documented? If the PR modifies an API (adding endpoints, changing request/response formats, deprecating functionality), is the API documentation updated to reflect the change?
- Are complex algorithms commented? Code should be self-documenting through clear naming and structure, but complex algorithms, non-obvious optimizations, and workarounds for external bugs deserve explanatory comments.
- Are configuration changes documented? If the PR adds new environment variables, feature flags, or configuration options, is there documentation explaining what they do, what the valid values are, and what the defaults are?
Code Quality
Code quality is about making the codebase easier to understand, modify, and maintain over time. High-quality code is not just correct — it is clear, consistent, and well-organized.
- Are names descriptive? Variables, functions, classes, and files should have names that clearly communicate their purpose. Avoid abbreviations that are not universally understood. A function called
calculateMonthlyRevenueis clearer thancalcMRev. - Is complexity managed? Functions should do one thing and do it well. If a function is longer than about 30 lines, or has more than 3 levels of nesting, consider whether it can be decomposed into smaller, more focused functions. High cyclomatic complexity makes code harder to test and easier to break.
- Is the DRY principle followed? Look for duplicated code that could be extracted into a shared function, component, or module. Duplication is a maintenance burden because changes need to be made in multiple places, and it is easy to miss one.
- Are SOLID principles applied? For object-oriented code, check that classes have a single responsibility, that code depends on abstractions rather than concrete implementations, and that extending functionality does not require modifying existing code.
- Is dead code removed? Commented-out code, unused imports, unreachable branches, and deprecated functions add noise and confusion. Remove them. Version control preserves history if you ever need to recover them.
- Is the code consistent? Does the new code follow the same patterns and conventions as the existing codebase? Consistency reduces the cognitive load of reading code because developers do not have to constantly adjust to different styles and approaches.
When to Use Checklists vs Judgment
Checklists are valuable tools, but they are not a substitute for human judgment. A checklist can remind you to check for SQL injection, but it cannot evaluate whether a particular architectural decision is appropriate for your project's specific constraints. It can prompt you to verify that tests exist, but it cannot judge whether those tests are meaningful.
Use checklists for the mechanical aspects of review: security patterns, accessibility requirements, performance anti-patterns, and coding conventions. These are the items that are easy to forget and have well-defined criteria for pass or fail. Use human judgment for the subjective aspects: architectural fit, design elegance, naming quality, and whether the approach is the right one for the problem.
The most effective teams use checklists as a foundation and judgment as a complement. The checklist ensures nothing important is overlooked. Judgment ensures that the code is not just correct but genuinely good.
Resources
- OWASP Code Review Guide — A comprehensive guide to security-focused code review from the Open Web Application Security Project
- The A11Y Project Checklist — A practical checklist for web accessibility covering WCAG criteria in plain language