code-review-checklist

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: Code works for happy path but fails on edge cases Solution: Ask "What if...?" questions

What if the input is null? What if the array is empty? What if the user is not authenticated? What if the network request fails? Problem: Security Vulnerabilities

Symptoms: Code exposes security risks Solution: Use security checklist

Run security scanners (npm audit, Snyk) Check OWASP Top 10 Validate all inputs Use parameterized queries Never trust user input Problem: Poor Test Coverage

Symptoms: New code has no tests or inadequate tests Solution: Require tests for all new code

Unit tests for functions Integration tests for features Edge case tests Error case tests Problem: Unclear Code

Symptoms: Reviewer can't understand what code does Solution: Request improvements

Better variable names Explanatory comments Smaller functions Clear structure Review Comment Templates Requesting Changes Issue: [Describe the problem]

Current code: ```javascript // Show problematic code ```

Suggested fix: ```javascript // Show improved code ```

Why: [Explain why this is better]

Asking Questions Question: [Your question]

Context: [Why you're asking]

Suggestion: [If you have one]

Praising Good Code Nice! [What you liked]

This is great because [explain why]

Related Skills @requesting-code-review - Prepare code for review @receiving-code-review - Handle review feedback @systematic-debugging - Debug issues found in review @test-driven-development - Ensure code has tests Additional Resources Google Code Review Guidelines OWASP Top 10 Code Review Best Practices How to Review Code

Pro Tip: Use a checklist template for every review to ensure consistency and thoroughness. Customize it for your team's specific needs!