code-review-checklist

Comprehensive checklist for conducting thorough code reviews covering functionality, security, performance, and maintainability

View Source
name:code-review-checklistdescription:"Comprehensive checklist for conducting thorough code reviews covering functionality, security, performance, and maintainability"

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!