#Code Review Standards & Guidelines
#Code Review Philosophy
- Why we do code reviews
- Goals: quality, knowledge sharing, consistency
- Blameless culture principles
#Review Process
#Before Submitting
- Self-review checklist
- Code follows style guide
- Tests are included
- Documentation is updated
- No debugging code left in
- Performance implications considered
#Submitting for Review
- PR/MR template to use
- Description requirements
- How to link related work
- Who to assign as reviewers
- When to ping for urgent review
#Review Expectations
- Expected review turnaround time
- Approval requirements (number of reviewers)
- CI/CD pipeline requirements
- When you can merge (approvals needed, tests passing)
#Code Quality Standards
- Language-specific style guide (link)
- Naming conventions
- Comment requirements
- Line length limits
#Testing Requirements
- Minimum code coverage percentage
- Unit test requirements
- Integration test requirements
- Edge case testing
- What not to test
#Documentation
- Docstring/comment requirements
- When to add/update README
- Changelog entries
- Architecture documentation updates needed
- Performance considerations to check
- Query optimization standards
- Caching approach review points
- Load testing requirements for significant changes
#Security
- Security review checklist
- Input validation requirements
- Authentication/authorization checks
- Sensitive data handling
- When to require security team review
#Must Fix
Items that block approval and must be addressed
Category: Security
- Example: "Sanitize user input before database query"
- Example: "Don't hardcode credentials"
Category: Correctness
- Example: "This logic doesn't handle null values"
- Example: "Off-by-one error in loop"
Category: Performance
- Example: "This query will cause N+1 problem"
- Example: "Consider caching this result"
#Should Fix
Items strongly recommended for improvement
- Example: "Could simplify this with list comprehension"
- Example: "This variable name is unclear"
#Nice to Have
Suggestions for improvement that can be addressed later
- Example: "Consider extracting this to a helper function"
- Example: "Could add a test for this edge case"
#Common Pitfalls to Avoid
As a Reviewer:
- Being nitpicky about style (use linters instead)
- Requesting changes that are stylistic preferences
- Not acknowledging good code
- Blocking on minor issues
- Not providing context for why changes are needed
As an Author:
- Submitting large PRs (should be < 400 lines)
- Mixing refactoring with feature work
- Not explaining the "why" in PR description
- Being defensive about feedback
- Merging without approval
#Escalation & Conflicts
#Disagreement on Approach
- When reviewer and author disagree on technical approach
- Process for resolution (discussion, escalate to tech lead, etc.)
- When to compromise vs. when to be firm
#Approval Override
- When can approvals be overridden?
- Who has authority?
- Documentation requirements
#Special Cases
#Hotfixes
- Approval requirements
- Documentation needed post-fix
- Expedited review process
#Dependency Updates
- Automatic vs. manual review
- Testing requirements
- Security patch handling
#Documentation-Only Changes
- Reduced review requirements
- Who needs to approve