Code Review Checklist

Perform thorough, consistent code reviews using structured checklists organized by priority and category.

When to Use This Skill

Reviewing pull requests and merge requests
Auditing existing code for quality issues
Establishing code review standards for a team
Training new developers on review practices
Creating pre-commit quality gates

Review Workflow

Phase 1: Preparation (2-5 minutes)

Understand the context
- Read the PR description and linked issues
- Understand the purpose and scope of changes
- Check if tests and documentation are included
Plan your review
- Estimate time needed based on change size
- Identify high-risk areas to focus on
- Note any domain expertise needed

Phase 2: Review (varies by size)

Architecture & Design - High-level structure
Security - Vulnerabilities and data protection
Correctness - Logic and edge cases
Performance - Efficiency concerns
Maintainability - Readability and organization
Testing - Coverage and quality
Documentation - Comments and docs

Phase 3: Feedback (5-10 minutes)

Organize findings by severity
Write constructive comments with suggestions
Acknowledge good practices
Provide overall recommendation

Severity Classification

Use these labels consistently:

Severity	Icon	Meaning	Action Required
Critical	🔴	Security vulnerabilities, data loss, crashes	Must fix before merge
High	🟠	Logic errors, resource leaks, missing validation	Should fix before merge
Medium	🟡	Performance issues, code smells, missing tests	Consider fixing
Low	🟢	Style issues, minor improvements	Optional, nice to have
Positive	✅	Good practices to acknowledge	Recognition

Review Checklists

🔐 Security Checklist

## Security Review

- [ ] **Input Validation**: All user inputs validated and sanitized
- [ ] **SQL Injection**: Parameterized queries used, no string concatenation
- [ ] **Command Injection**: Shell commands properly escaped or avoided
- [ ] **XSS Prevention**: Output properly encoded for context
- [ ] **Authentication**: Auth checks present on protected resources
- [ ] **Authorization**: Permission checks for sensitive operations
- [ ] **Secrets Management**: No hardcoded credentials or API keys
- [ ] **Cryptography**: Strong algorithms used (no MD5/SHA1 for security)
- [ ] **HTTPS**: Secure transport for sensitive data
- [ ] **Dependencies**: No known vulnerabilities in dependencies
- [ ] **Error Messages**: No sensitive info leaked in errors
- [ ] **Logging**: No PII or secrets in logs

✓ Correctness Checklist

## Correctness Review

- [ ] **Logic**: Algorithm correctly implements requirements
- [ ] **Edge Cases**: Empty inputs, nulls, boundaries handled
- [ ] **Error Handling**: Exceptions caught and handled appropriately
- [ ] **Return Values**: All code paths return expected types
- [ ] **State Management**: Mutable state handled safely
- [ ] **Concurrency**: Race conditions and deadlocks prevented
- [ ] **Data Validation**: Invalid data rejected early
- [ ] **Resource Cleanup**: Files, connections, locks properly released
- [ ] **Idempotency**: Operations safe to retry where needed
- [ ] **Transactions**: Database operations properly atomic

⚡ Performance Checklist

## Performance Review

- [ ] **Algorithm Complexity**: Appropriate O(n) for data size
- [ ] **Database Queries**: N+1 queries avoided, proper indexing
- [ ] **Caching**: Expensive operations cached where beneficial
- [ ] **Lazy Loading**: Large data loaded on demand
- [ ] **Batch Operations**: Bulk inserts/updates used when possible
- [ ] **Memory Usage**: No memory leaks, large objects released
- [ ] **Network Calls**: Minimized, parallelized where possible
- [ ] **Pagination**: Large result sets paginated
- [ ] **Async Operations**: I/O-bound work non-blocking
- [ ] **Resource Pools**: Connection/thread pools properly sized

🏗️ Architecture & Design Checklist

## Design Review

- [ ] **Single Responsibility**: Each class/function does one thing
- [ ] **Open/Closed**: Extensible without modification
- [ ] **Dependency Inversion**: Depends on abstractions, not concretions
- [ ] **DRY**: No duplicated code or logic
- [ ] **KISS**: Solution is as simple as possible
- [ ] **Separation of Concerns**: Layers properly separated
- [ ] **Coupling**: Dependencies minimized between modules
- [ ] **Cohesion**: Related functionality grouped together
- [ ] **Abstraction Level**: Consistent within each layer
- [ ] **Backwards Compatibility**: Breaking changes documented

📖 Maintainability Checklist

## Maintainability Review

- [ ] **Naming**: Variables, functions, classes clearly named
- [ ] **Function Length**: Functions under 30 lines
- [ ] **Cyclomatic Complexity**: Under 10 per function
- [ ] **Nesting Depth**: Maximum 3-4 levels
- [ ] **Magic Numbers**: Constants extracted and named
- [ ] **Comments**: Complex logic explained, no obvious comments
- [ ] **Type Hints**: Present on public APIs
- [ ] **Docstrings**: Public functions documented
- [ ] **Code Organization**: Logical file and folder structure
- [ ] **Formatting**: Consistent style (automated preferred)

🧪 Testing Checklist

## Testing Review

- [ ] **Coverage**: New code has tests (target >80%)
- [ ] **Unit Tests**: Business logic tested in isolation
- [ ] **Integration Tests**: Component interactions tested
- [ ] **Edge Cases**: Boundary conditions and errors tested
- [ ] **Assertions**: Meaningful, specific assertions
- [ ] **Test Isolation**: Tests don't depend on each other
- [ ] **Test Clarity**: Test names describe expected behavior
- [ ] **Mocking**: External dependencies properly mocked
- [ ] **Performance Tests**: Critical paths benchmarked
- [ ] **Regression Tests**: Bug fixes include tests

📝 Documentation Checklist

## Documentation Review

- [ ] **README**: Updated if setup/usage changed
- [ ] **API Docs**: Endpoints, parameters, responses documented
- [ ] **Changelog**: Notable changes recorded
- [ ] **Migration Guide**: Breaking changes explained
- [ ] **Architecture Docs**: Significant design changes documented
- [ ] **Comments**: Complex algorithms explained
- [ ] **Examples**: Usage examples for new features
- [ ] **Error Messages**: User-facing errors helpful

Review Output Template

Use this template for review summaries:

## Code Review Summary

**PR**: #{number} - {title}
**Reviewer**: {name}
**Date**: {date}

### Overview
- **Files Changed**: {count}
- **Lines Added/Removed**: +{added} / -{removed}
- **Overall Quality**: {score}/10
- **Recommendation**: {Approve | Request Changes | Needs Discussion}

### 🔴 Critical Issues
{List issues that must be fixed}

### 🟠 High Priority
{List significant issues}

### 🟡 Medium Priority
{List improvements to consider}

### 🟢 Suggestions
{List minor enhancements}

### ✅ Good Practices Observed
{Acknowledge well-written code}

### Summary
{Overall assessment with prioritized next steps}

Review Comment Patterns

Constructive Feedback Format

**Issue**: [Brief description]
**Location**: `file.py:42`
**Severity**: 🟠 High

**Current code**:
```python
result = db.query(f"SELECT * FROM users WHERE id = {user_id}")

Problem: SQL injection vulnerability - user input directly in query.

Suggestion:

result = db.query("SELECT * FROM users WHERE id = ?", [user_id])

Reference: OWASP SQL Injection


### Positive Feedback Format

```markdown
✅ **Nice work!**

Good use of the strategy pattern here for payment processing.
This makes it easy to add new payment providers without modifying existing code.

Question Format

❓ **Question**

I see this uses eager loading for all associations.
Was this intentional? For this use case, lazy loading might improve initial page load.

Language-Specific Considerations

Python

Type hints on all public functions
Pydantic for data validation
Context managers for resources
async/await for I/O operations

JavaScript/TypeScript

Strict TypeScript mode enabled
Proper null/undefined handling
No floating promises
Memory leak prevention in closures

SQL

Parameterized queries only
Index usage verified
Transaction boundaries defined
Explain plan reviewed for complex queries

Resources

Guidelines

Be kind: Critique code, not people
Be specific: Reference exact lines and provide examples
Be helpful: Suggest solutions, not just problems
Be timely: Review within 24 hours when possible
Be thorough: Check all categories, not just obvious issues
Be balanced: Acknowledge good work alongside issues

code-review-checklist