Code Review Checklist

Overview

Provide a systematic checklist for conducting thorough code reviews. This skill helps reviewers ensure code quality, catch bugs, identify security issues, and maintain consistency across the codebase.

When to Use This Skill

• Use when reviewing pull requests
• Use when conducting code audits
• Use when establishing code review standards for a team
• Use when training new developers on code review practices
• Use when you want to ensure nothing is missed in reviews
• Use when creating code review documentation

How It Works

Step 1: Understand the Context

Before reviewing code, I'll help you understand:

• What problem does this code solve?
• What are the requirements?
• What files were changed and why?
• Are there related issues or tickets?
• What's the testing strategy?

Step 2: Review Functionality

Check if the code works correctly:

• Does it solve the stated problem?
• Are edge cases handled?
• Is error handling appropriate?
• Are there any logical errors?
• Does it match the requirements?

Step 3: Review Code Quality

Assess code maintainability:

• Is the code readable and clear?
• Are names descriptive?
• Is it properly structured?
• Are functions/methods focused?
• Is there unnecessary complexity?

Step 4: Review Security

Check for security issues:

• Are inputs validated?
• Is sensitive data protected?
• Are there SQL injection risks?
• Is authentication/authorization correct?
• Are dependencies secure?

Step 5: Review Performance

Look for performance issues:

• Are there unnecessary loops?
• Is database access optimized?
• Are there memory leaks?
• Is caching used appropriately?
• Are there N+1 query problems?

Step 6: Review Tests

Verify test coverage:

• Are there tests for new code?
• Do tests cover edge cases?
• Are tests meaningful?
• Do all tests pass?
• Is test coverage adequate?

Examples

Example 1: Functionality Review Checklist

## Functionality Review

### Requirements
- [ ] Code solves the stated problem
- [ ] All acceptance criteria are met
- [ ] Edge cases are handled
- [ ] Error cases are handled
- [ ] User input is validated

### Logic
- [ ] No logical errors or bugs
- [ ] Conditions are correct (no off-by-one errors)
- [ ] Loops terminate correctly
- [ ] Recursion has proper base cases
- [ ] State management is correct

### Error Handling
- [ ] Errors are caught appropriately
- [ ] Error messages are clear and helpful
- [ ] Errors don't expose sensitive information
- [ ] Failed operations are rolled back
- [ ] Logging is appropriate

### Example Issues to Catch:

**❌ Bad - Missing validation:**
\`\`\`javascript
function createUser(email, password) {
  // No validation!
  return db.users.create({ email, password });
}
\`\`\`

**✅ Good - Proper validation:**
\`\`\`javascript
function createUser(email, password) {
  if (!email || !isValidEmail(email)) {
    throw new Error('Invalid email address');
  }
  if (!password || password.length < 8) {
    throw new Error('Password must be at least 8 characters');
  }
  return db.users.create({ email, password });
}
\`\`\`

Example 2: Security Review Checklist

## Security Review

### Input Validation
- [ ] All user inputs are validated
- [ ] SQL injection is prevented (use parameterized queries)
- [ ] XSS is prevented (escape output)
- [ ] CSRF protection is in place
- [ ] File uploads are validated (type, size, content)

### Authentication & Authorization
- [ ] Authentication is required where needed
- [ ] Authorization checks are present
- [ ] Passwords are hashed (never stored plain text)
- [ ] Sessions are managed securely
- [ ] Tokens expire appropriately

### Data Protection
- [ ] Sensitive data is encrypted
- [ ] API keys are not hardcoded
- [ ] Environment variables are used for secrets
- [ ] Personal data follows privacy regulations
- [ ] Database credentials are secure

### Dependencies
- [ ] No known vulnerable dependencies
- [ ] Dependencies are up to date
- [ ] Unnecessary dependencies are removed
- [ ] Dependency versions are pinned

### Example Issues to Catch:

**❌ Bad - SQL injection risk:**
\`\`\`javascript
const query = \`SELECT * FROM users WHERE email = '\${email}'\`;
db.query(query);
\`\`\`

**✅ Good - Parameterized query:**
\`\`\`javascript
const query = 'SELECT * FROM users WHERE email = $1';
db.query(query, [email]);
\`\`\`

**❌ Bad - Hardcoded secret:**
\`\`\`javascript
const API_KEY = 'sk_live_abc123xyz';
\`\`\`

**✅ Good - Environment variable:**
\`\`\`javascript
const API_KEY = process.env.API_KEY;
if (!API_KEY) {
  throw new Error('API_KEY environment variable is required');
}
\`\`\`

Example 3: Code Quality Review Checklist

## Code Quality Review

### Readability
- [ ] Code is easy to understand
- [ ] Variable names are descriptive
- [ ] Function names explain what they do
- [ ] Complex logic has comments
- [ ] Magic numbers are replaced with constants

### Structure
- [ ] Functions are small and focused
- [ ] Code follows DRY principle (Don't Repeat Yourself)
- [ ] Proper separation of concerns
- [ ] Consistent code style
- [ ] No dead code or commented-out code

### Maintainability
- [ ] Code is modular and reusable
- [ ] Dependencies are minimal
- [ ] Changes are backwards compatible
- [ ] Breaking changes are documented
- [ ] Technical debt is noted

### Example Issues to Catch:

**❌ Bad - Unclear naming:**
\`\`\`javascript
function calc(a, b, c) {
  return a * b + c;
}
\`\`\`

**✅ Good - Descriptive naming:**
\`\`\`javascript
function calculateTotalPrice(quantity, unitPrice, tax) {
  return quantity * unitPrice + tax;
}
\`\`\`

**❌ Bad - Function doing too much:**
\`\`\`javascript
function processOrder(order) {
  // Validate order
  if (!order.items) throw new Error('No items');
  
  // Calculate total
  let total = 0;
  for (let item of order.items) {
    total += item.price * item.quantity;
  }
  
  // Apply discount
  if (order.coupon) {
    total *= 0.9;
  }
  
  // Process payment
  const payment = stripe.charge(total);
  
  // Send email
  sendEmail(order.email, 'Order confirmed');
  
  // Update inventory
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

**✅ Good - Separated concerns:**
\`\`\`javascript
function processOrder(order) {
  validateOrder(order);
  const total = calculateOrderTotal(order);
  const payment = processPayment(total);
  sendOrderConfirmation(order.email);
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

Best Practices

✅ Do This

• Review Small Changes - Smaller PRs are easier to review thoroughly
• Check Tests First - Verify tests pass and cover new code
• Run the Code - Test it locally when possible
• Ask Questions - Don't assume, ask for clarification
• Be Constructive - Suggest improvements, don't just criticize
• Focus on Important Issues - Don't nitpick minor style issues
• Use Automated Tools - Linters, formatters, security scanners
• Review Documentation - Check if docs are updated
• Consider Performance - Think about scale and efficiency
• Check for Regressions - Ensure existing functionality still works

❌ Don't Do This

• Don't Approve Without Reading - Actually review the code
• Don't Be Vague - Provide specific feedback with examples
• Don't Ignore Security - Security issues are critical
• Don't Skip Tests - Untested code will cause problems
• Don't Be Rude - Be respectful and professional
• Don't Rubber Stamp - Every review should add value
• Don't Review When Tired - You'll miss important issues
• Don't Forget Context - Understand the bigger picture

Complete Review Checklist

Pre-Review

• Read the PR description and linked issues
• Understand what problem is being solved
• Check if tests pass in CI/CD
• Pull the branch and run it locally

Functionality

• Code solves the stated problem
• Edge cases are handled
• Error handling is appropriate
• User input is validated
• No logical errors

Security

• No SQL injection vulnerabilities
• No XSS vulnerabilities
• Authentication/authorization is correct
• Sensitive data is protected
• No hardcoded secrets

Performance

• No unnecessary database queries
• No N+1 query problems
• Efficient algorithms used
• No memory leaks
• Caching used appropriately

Code Quality

• Code is readable and clear
• Names are descriptive
• Functions are focused and small
• No code duplication
• Follows project conventions

Tests

• New code has tests
• Tests cover edge cases
• Tests are meaningful
• All tests pass
• Test coverage is adequate

Documentation

• Code comments explain why, not what
• API documentation is updated
• README is updated if needed
• Breaking changes are documented
• Migration guide provided if needed

Git

• Commit messages are clear
• No merge conflicts
• Branch is up to date with main
• No unnecessary files committed
• .gitignore is properly configured

Common Pitfalls

Problem: Missing Edge Cases

Symptoms: C