Code Review Snippets
This section provides guidance on code review checklists, code smells, secure coding practices, vulnerability checks, and antipatterns.
Philosophy
Ethics in code review and a clear code of conduct are crucial for maintaining a respectful and constructive environment between developers and reviewers. Mutual professionalism helps ensure that feedback is focused on code quality and security, rather than personal criticism, fostering growth and collaboration within the team.
Checklists are important tools for consistency and thoroughness in reviewsβthey help prevent common mistakes from being overlooked. However, treat these lists as flexible frameworks, not rigid rules. Every project and context is different. Use this guide as a template to drive helpful discussions and effective reviews, not as a set of mandatory steps to follow verbatim. Thinking critically about context and applying judgment is always encouraged.
Layered Review Approach
Effective code reviews follow a structured approach, progressing from critical issues to refinements:
Layer 1: Immediate Rejection Criteria (5 minutes)
These issues should cause immediate rejection without deep review. Fix first, then resubmit:
- π« CI/CD failures - All automated checks must pass
- π« No tests - New functionality requires tests
- π« Security vulnerabilities - SQL injection, XSS, hardcoded secrets
- π« Hardcoded credentials - API keys, passwords, tokens in code
- π« Massive PR - >1000 lines without justification (split it up)
- π« Breaking changes - No migration plan or documentation
- π« Merge conflicts - Must be resolved before review
Action: Request changes immediately. Don't waste time on deeper review until these are fixed.
Layer 2: Functionality & Correctness (15-30 minutes)
Once Layer 1 passes, verify the code actually works:
- β Does it meet the requirements?
- β Does the logic make sense?
- β Are edge cases handled?
- β Is error handling appropriate?
- β Are there obvious bugs?
Action: Test the code if possible. Run it locally. Think through scenarios.
Layer 3: Security & Data Safety (10-20 minutes)
Critical for production code:
- π Input validation
- π Output sanitization
- π Authentication/authorization
- π Data exposure risks
- π Dependency vulnerabilities
Action: Think like an attacker. What could go wrong?
Layer 4: Architecture & Design (10-15 minutes)
Does it fit the system?
- ποΈ Follows existing patterns?
- ποΈ Proper separation of concerns?
- ποΈ Appropriate dependencies?
- ποΈ No tight coupling?
- ποΈ Scalability considerations?
Action: Zoom out. How does this fit the bigger picture?
Layer 5: Code Quality & Maintainability (10-15 minutes)
Will future developers (including the author) understand this?
- π Clear naming
- π Readable code structure
- π Appropriate comments
- π No code duplication
- π Functions are focused
Action: Read the code as if you'll maintain it in 6 months.
Layer 6: Tests & Documentation (5-10 minutes)
Supporting artifacts:
- π Tests are comprehensive
- π Tests are readable
- π Documentation is updated
- π Complex logic is explained
Action: Could someone else understand and extend this?
Layer 7: Performance & Optimization (5-10 minutes)
Only if relevant to the change:
- β‘ No obvious performance issues
- β‘ Efficient algorithms
- β‘ Database queries optimized
- β‘ Caching where appropriate
Action: Is this fast enough? Will it scale?
Layer 8: Nitpicks & Style (5 minutes)
Last and least important:
- π¨ Code style (should be automated)
- π¨ Minor naming preferences
- π¨ Formatting (should be automated)
Action: Only mention if not covered by linters. Use [NITPICK] prefix.
Review Timing Guidelines
Immediate Rejection Criteria - Detailed
π« CI/CD Failures
Why: If automated checks fail, the code isn't ready for human review.
Examples:
- Linting errors
- Test failures
- Build failures
- Security scans failing
Response: "Please fix CI failures before requesting review."
π« No Tests
Why: Untested code is a liability. Tests are not optional.
Exceptions:
- Pure documentation changes
- Configuration-only changes
- Hotfixes (but tests should follow immediately)
Response: "Please add tests for the new functionality before I review."
π« Security Vulnerabilities
Why: Security issues can't wait. They're critical.
Examples:
- SQL injection:
query = "SELECT * FROM users WHERE id = " + userId - XSS: Unescaped user input in HTML
- Hardcoded secrets:
API_KEY = "sk_live_123456" - Missing authentication checks
Response: "[BLOCKER] Security issue: [specific vulnerability]. Must be fixed."
π« Massive PRs (>1000 lines)
Why: Large PRs are impossible to review thoroughly. Bugs hide in large diffs.
Exceptions:
- Generated code (migrations, API clients)
- Large refactors (but should be discussed first)
- Dependency updates
Response: "This PR is too large to review effectively. Please split into smaller PRs."
π« Breaking Changes Without Migration
Why: Breaking changes without a plan cause production incidents.
Required:
- Migration guide
- Deprecation warnings
- Backward compatibility period
- Communication plan
Response: "Breaking changes require a migration plan. Please document the upgrade path."
Best Practices Summary
For Reviewers
- Start with Layer 1 - Reject immediately if criteria not met
- Work through layers - Don't skip to nitpicks
- Be constructive - Explain why, not just what
- Prioritize issues - Use [BLOCKER], [IMPORTANT], [SUGGESTION] prefixes
- Approve or request changes - Don't leave PRs hanging
- Review within 24 hours - Don't block the team
For Developers
- Self-review first - Catch obvious issues yourself
- Run all checks locally - Don't rely on CI to catch everything
- Keep PRs small - <400 lines is ideal
- Write clear descriptions - Help reviewers understand context
- Respond to feedback - Address all comments
- Don't take it personally - Reviews are about code, not you
Related Topics
- Developer Checklist - What to check before submitting
- Reviewer Checklist - What to look for when reviewing
- Code Smells - Common issues to watch for
- Secure Coding - Security best practices
- Antipatterns - Common mistakes to avoid
Snippets
- C/C++ Code Smells
Common code smells in C/C++ and how to fix them. Memory Leaks 1// β Bad 2void β¦ - C/C++ Secure Coding
Secure coding practices for C/C++ applications. Buffer Overflow Prevention 1// β β¦ - C/C++ Vulnerability Checks
Tools for checking vulnerabilities in C/C++ code. Valgrind 1# Install 2sudo apt β¦ - Common Antipatterns
Common software antipatterns to avoid across all languages and architectures. β¦ - Common Code Smells
Common code smells to watch for during code reviews with examples and fixes. β¦ - Developer Pre-Submission Checklist
Comprehensive checklist for developers before submitting a pull request. Code β¦ - Go Code Smells
Common code smells in Go and how to fix them. Ignoring Errors 1// β Bad 2result, β¦ - Go Secure Coding
Secure coding practices for Go applications. SQL Injection Prevention 1// β β¦ - Go Vulnerability Checks
Tools for checking vulnerabilities in Go code. Govulncheck 1# Install 2go β¦ - Haskell Code Smells
Common code smells in Haskell and how to fix them. Partial Functions 1-- β Bad: β¦ - Haskell Secure Coding
Secure coding practices for Haskell applications. SQL Injection Prevention 1-- β β¦ - Haskell Vulnerability Checks
Tools for checking vulnerabilities in Haskell code. Cabal Outdated 1# Check β¦ - Python Code Smells
Common code smells in Python and how to fix them. Mutable Default Arguments 1# β β¦ - Python Secure Coding
Secure coding practices for Python applications. SQL Injection Prevention 1# β β¦ - Python Vulnerability Checks
Tools for checking vulnerabilities in Python code. Safety - Dependency Scanner β¦ - Reviewer Checklist
Comprehensive checklist for code reviewers to ensure thorough and constructive β¦ - Rust Code Smells
Common code smells in Rust and how to fix them. Unwrap/Expect Abuse 1// β Bad β¦ - Rust Secure Coding
Secure coding practices for Rust applications. SQL Injection Prevention 1// β β¦ - Rust Vulnerability Checks
Tools for checking vulnerabilities in Rust code. Cargo Audit 1# Install 2cargo β¦ - TypeScript Code Smells
Common code smells in TypeScript and how to fix them. Using any 1// β Bad β¦ - TypeScript Secure Coding
Secure coding practices for TypeScript applications. XSS Prevention 1// β β¦ - TypeScript Vulnerability Checks
Tools for checking vulnerabilities in TypeScript/JavaScript code. npm audit 1# β¦