Code Review Best Practices for Engineering Teams
Code review is one of the highest-leverage practices in software engineering. A thorough review catches bugs before they reach production, spreads knowledge across the team, improves code consistency, and helps junior developers learn. But code review is only effective when done right — rushed reviews, massive pull requests, and personal criticism undermine the benefits. This article outlines best practices for both authors and reviewers.
Keep Pull Requests Small
The single most important factor in review quality is PR size. Studies show that code review effectiveness drops dramatically once a PR exceeds 400 lines of code. Small PRs (under 200 lines) are reviewed more thoroughly, catch more bugs, and ship faster. Break large features into a sequence of small, logically independent PRs — each one should add one coherent change. If you are refactoring, do not mix refactoring with feature work in the same PR. Use draft PRs for work-in-progress to get early design feedback without pressure. A good PR description explains what the change does, why it is needed, and how it was tested.
## Description
Add user authentication with JWT tokens
## Changes
- Add JWT token generation and validation
- Add login endpoint (POST /api/auth/login)
- Add token verification middleware to protected routes
## Testing
- [x] Unit tests for token generation and validation
- [x] Integration test for login flow
- [x] Test expired token rejection
## Related Issues
Closes #142What to Look For in a Review
A good code review covers multiple dimensions. Correctness: does the code handle the requirements, including edge cases like empty states, null values, and error responses? Security: are there SQL injection vectors, XSS vulnerabilities, or hardcoded secrets? Performance: are there N+1 queries, unbounded list comprehensions, or obvious inefficiencies? Testability: are the functions testable in isolation, or are they tightly coupled to concrete dependencies? Readability: are the variable and function names descriptive? Is the control flow clear? Would a new team member understand this code? Focus on correctness and security first — style preferences are less important and can be enforced by automated formatters like Black, Ruff, or Prettier.
Automate Before Human Review
Set up CI to run linters, formatters, type checkers, and tests before a reviewer looks at the code. This frees human reviewers to focus on high-level concerns — design, correctness, and architecture — rather than nitpicking formatting or missing type annotations. Use a pre-commit configuration file so developers catch issues locally before pushing. GitHub Actions, GitLab CI, and Jenkins can all enforce these checks as required status checks that must pass before merging. A typical pre-commit config includes hooks for trailing whitespace, YAML validation, Python import sorting, and code formatting.
# .github/PULL_REQUEST_TEMPLATE.md
## Description
Briefly describe the change and why it is needed.
## Testing
- [ ] Unit tests pass
- [ ] Integration tests pass
- [ ] Manual testing performed
## Deployment Notes
Any migration steps, environment variables, or rollback considerations.Checklists for Common Change Types
Different change types need different review focus. For a database migration PR, check for backward compatibility, rollback scripts, and performance impact on large tables. For a security-related change, look for input validation, authentication checks, and proper error handling that does not leak sensitive information. For a UI change, verify accessibility (keyboard navigation, screen reader support, color contrast according to WCAG 2.1 AA), loading states, and error messages. For an API change, ensure versioning is considered, deprecated fields are removed only after a transition period, and the OpenAPI spec is updated to reflect the changes.
The Review Process and Giving Feedback
A good review process has a clear workflow. The author creates a PR with a descriptive title and summary, checks CI passes, and assigns reviewers (typically 1-2 for simple changes, more for complex architectural decisions). Reviewers should respond within 24 hours — block time in your calendar for reviews just as you would for any other task. If a PR sits for days, context is lost and the author has to context-switch back to remember what they were doing. Use GitHub’s request changes, comment, and approve features appropriately. Frame feedback as questions rather than commands: instead of “Change this to use dependency injection,” say “Would dependency injection make this easier to test?” This invites discussion and acknowledges there may be context the reviewer does not have. Separate the code from the developer — critique the code, not the person. When receiving feedback, treat it as a learning opportunity. Not every comment needs to be addressed if there is a reasoned justification against it, but be open to changing your approach. If a reviewer does not understand your code, that is often a sign that the code needs better naming or documentation rather than a failing of the reviewer.