Code reviews ensure code quality, knowledge sharing, and maintain consistency across our codebase. Every change should be reviewed before merging.
- Functionality: Does the code do what it's supposed to do?
- Design: Is the code well-designed and appropriate for our system?
- Complexity: Could the code be simpler?
- Tests: Does the code have appropriate test coverage?
- Naming: Are variables, methods, and classes named clearly?
- Comments: Are comments clear and useful?
- Documentation: Is documentation updated if needed?
- Style: Does the code follow our style guidelines?
- Security vulnerabilities
- Performance problems
- Memory leaks
- Race conditions
- Error handling gaps
- Breaking changes
- DRY (Don't Repeat Yourself) violations
- SOLID principle violations
- Unnecessary complexity
- Magic numbers or strings
- Poor error messages
- Missing edge case handling
- [BLOCKING]: Must be fixed before merge
- [SUGGESTION]: Consider this improvement
- [QUESTION]: Seeking clarification
- [NIT]: Minor style issue (optional fix)
- [PRAISE]: Highlighting good practices
❌ "This is wrong" ✅ "This could cause a null pointer exception. Consider adding a null check here."
❌ "Bad variable name" ✅ "Consider renaming 'data' to 'userProfileData' for clarity"
- Respond to all comments
- Mark resolved conversations
- Explain your reasoning when disagreeing
- Ask for clarification when needed
- Thank reviewers for their time
- Create new commits for review feedback (don't force-push during review)
- Reference reviewer comments in commit messages when applicable
- Re-request review after addressing feedback
- Initial review: Within 24 hours of request
- Follow-up reviews: Within 12 hours
- Response to feedback: Within 24 hours
- Urgent fixes: Within 2 hours (mark as urgent)
- Use inline comments for specific line feedback
- Use general PR comments for overall feedback
- Approve, Request Changes, or Comment appropriately
- Use suggested changes feature when possible
- Be respectful and constructive
- Focus on the code, not the person
- Explain the "why" behind your suggestions
- Acknowledge good solutions
- Learn from the code you review
- Don't approve if you don't understand
- Consider the bigger picture
- Balance perfectionism with pragmatism
- Approving without thorough review
- Nitpicking on style when auto-formatters exist
- Blocking on personal preferences
- Reviewing only your area of expertise
- Delaying reviews unnecessarily
- Being overly critical or harsh
If consensus cannot be reached:
- Discuss in team channel
- Involve tech lead for tie-breaking
- Document decision for future reference