Code Review Best Practices and Guidelines

Introduction

Code review is one of the most effective practices for improving code quality, sharing knowledge, and building better software teams. It’s a systematic examination of source code intended to find and fix mistakes overlooked in the initial development phase, improving both the quality of the code and the skills of the developers involved.

In this comprehensive guide, we’ll explore code review best practices, common pitfalls, effective feedback techniques, and tools that can help streamline the process.

Why Code Reviews Matter

Benefits of Code Reviews

Quality Improvement

Code reviews catch bugs, design issues, and security vulnerabilities before they reach production. Studies show that code reviews can catch 60-90% of defects in code.

Knowledge Sharing

Reviews spread knowledge across the team about different parts of the codebase, programming techniques, and business logic.

Collective Code Ownership

When multiple team members review code, the entire team shares responsibility for the codebase, reducing bus factor and knowledge silos.

Mentorship and Learning

Junior developers learn from seniors through feedback, while seniors learn new approaches and stay aware of emerging patterns.

Consistency

Reviews help maintain coding standards and architectural consistency across the codebase.

Team Building

Constructive code reviews foster collaboration and improve team dynamics when done right.

The Code Review Process

Before Submitting Code for Review

1. Self-Review First

# Review your own changes before submitting
git diff main...your-branch

# Check for common issues
git diff --check  # Trailing whitespace, etc.

# Review commit history
git log --oneline -10

2. Run Tests Locally

# Ensure all tests pass
npm test
pytest
mvn test

# Run linters
eslint .
flake8 .
dotnet format --verify-no-changes

3. Write Clear Commit Messages

# Good commit message
git commit -m "Add user authentication middleware

- Implement JWT token validation
- Add rate limiting for login endpoints
- Include error handling for expired tokens

Fixes #123"

# Bad commit message
git commit -m "fixed stuff"

4. Keep Pull Requests Small

Aim for:

• 200-400 lines of code changes
• Single feature or bug fix
• Clear, focused scope
• Complete implementation

5. Provide Context

Create a detailed pull request description:

## Description
Implements user authentication using JWT tokens

## Changes Made
- Added JWT middleware for route protection
- Implemented token refresh mechanism
- Added unit tests for authentication flow
- Updated API documentation

## Testing
- Unit tests: 95% coverage
- Manual testing on staging environment
- Tested with expired tokens and invalid credentials

## Screenshots
[If applicable]

## Related Issues
Closes #123
Related to #124

During the Review

For Reviewers

1. Review Process Checklist

# Python - Code Review Checklist Implementation
from dataclasses import dataclass
from typing import List
from enum import Enum

class ReviewCategory(Enum):
    FUNCTIONALITY = "functionality"
    CODE_QUALITY = "code_quality"
    PERFORMANCE = "performance"
    SECURITY = "security"
    TESTING = "testing"
    DOCUMENTATION = "documentation"

@dataclass
class ReviewItem:
    category: ReviewCategory
    question: str
    passed: bool
    notes: str = ""

class CodeReviewChecklist:
    def __init__(self):
        self.items: List[ReviewItem] = []
    
    def add_item(self, category: ReviewCategory, question: str, 
                 passed: bool, notes: str = ""):
        self.items.append(ReviewItem(category, question, passed, notes))
    
    def get_summary(self) -> dict:
        total = len(self.items)
        passed = sum(1 for item in self.items if item.passed)
        
        by_category = {}
        for item in self.items:
            cat = item.category.value
            if cat not in by_category:
                by_category[cat] = {"total": 0, "passed": 0}
            by_category[cat]["total"] += 1
            if item.passed:
                by_category[cat]["passed"] += 1
        
        return {
            "total_items": total,
            "passed_items": passed,
            "pass_rate": (passed / total * 100) if total > 0 else 0,
            "by_category": by_category
        }
    
    def get_failed_items(self) -> List[ReviewItem]:
        return [item for item in self.items if not item.passed]

# Usage
checklist = CodeReviewChecklist()

# Functionality checks
checklist.add_item(
    ReviewCategory.FUNCTIONALITY,
    "Does the code do what it's supposed to do?",
    True
)
checklist.add_item(
    ReviewCategory.FUNCTIONALITY,
    "Are edge cases handled properly?",
    False,
    "Missing null check in getUserById"
)

# Code quality checks
checklist.add_item(
    ReviewCategory.CODE_QUALITY,
    "Is the code readable and maintainable?",
    True
)
checklist.add_item(
    ReviewCategory.CODE_QUALITY,
    "Are naming conventions followed?",
    True
)

# Security checks
checklist.add_item(
    ReviewCategory.SECURITY,
    "Is user input properly validated?",
    True
)
checklist.add_item(
    ReviewCategory.SECURITY,
    "Are there any SQL injection risks?",
    True
)

# Testing checks
checklist.add_item(
    ReviewCategory.TESTING,
    "Are there sufficient unit tests?",
    False,
    "Missing tests for error scenarios"
)

summary = checklist.get_summary()
print(f"Pass rate: {summary['pass_rate']:.1f}%")

failed = checklist.get_failed_items()
for item in failed:
    print(f"[{item.category.value}] {item.question}")
    print(f"  Notes: {item.notes}")

2. What to Look For

Functionality

• Does the code work as intended?
• Are edge cases handled?
• Is error handling appropriate?
• Are there any logical errors?

Code Quality

• Is the code readable?
• Are names descriptive?
• Is there duplication?
• Is complexity reasonable?
• Does it follow team conventions?

Performance

• Are there performance issues?
• Are algorithms efficient?
• Is there unnecessary work?
• Are resources properly managed?

Security

• Is input validated?
• Are there injection vulnerabilities?
• Is authentication/authorization correct?
• Are secrets hardcoded?

Testing

• Are there adequate tests?
• Do tests cover edge cases?
• Are tests meaningful?
• Is test code quality good?

Documentation

• Is code self-documenting?
• Are complex parts explained?
• Is API documentation updated?
• Are README files current?

Review Techniques

The Layered Review Approach

// JavaScript - Example of what to review at each layer

// Layer 1: High-Level Review (5-10 minutes)
// - Overall design and architecture
// - PR description and context
// - File changes overview

class UserService {
  constructor(userRepository, emailService) {
    this.userRepository = userRepository;
    this.emailService = emailService;
  }
  
  // First pass: Does the overall structure make sense?
  // Are dependencies injected properly?
  // Is this the right place for this functionality?
}

// Layer 2: Implementation Review (15-30 minutes)
// - Logic correctness
// - Error handling
// - Edge cases

class UserService {
  async createUser(userData) {
    // Check: Is validation comprehensive?
    if (!userData.email || !userData.password) {
      throw new Error('Email and password required');
    }
    
    // Check: Is password handling secure?
    const hashedPassword = await bcrypt.hash(userData.password, 10);
    
    // Check: Is error handling appropriate?
    try {
      const user = await this.userRepository.create({
        ...userData,
        password: hashedPassword
      });
      
      // Check: Is this async operation handled correctly?
      await this.emailService.sendWelcome(user.email);
      
      return user;
    } catch (error) {
      // Check: Is error information sufficient?
      throw new Error(`Failed to create user: ${error.message}`);
    }
  }
}

// Layer 3: Detail Review (10-20 minutes)
// - Code style
// - Naming
// - Comments
// - Minor improvements

class UserService {
  async createUser(userData) {
    // Good: Descriptive function name
    // Good: Clear parameter name
    
    this._validateUserData(userData);  // Good: Extracted validation
    
    const hashedPassword = await this._hashPassword(userData.password);
    // Good: Extracted password hashing
    
    try {
      const user = await this.userRepository.create({
        email: userData.email,
        password: hashedPassword,
        createdAt: new Date()  // Good: Explicit timestamp
      });
      
      // Consider: Should email sending be async/decoupled?
      await this.emailService.sendWelcome(user.email);
      
      return user;
    } catch (error) {
      // Consider: Should we log this error?
      throw new Error(`Failed to create user: ${error.message}`);
    }
  }
  
  _validateUserData(userData) {
    // Good: Private method naming convention
    if (!userData.email || !this._isValidEmail(userData.email)) {
      throw new Error('Valid email required');
    }
    
    if (!userData.password || userData.password.length < 8) {
      throw new Error('Password must be at least 8 characters');
    }
  }
  
  _isValidEmail(email) {
    const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
    return emailRegex.test(email);
  }
  
  async _hashPassword(password) {
    return await bcrypt.hash(password, 10);
  }
}

Giving Effective Feedback

Principles of Good Feedback

1. Be Specific and Actionable

# Bad feedback
"This code is bad"

# Good feedback
"""
The function `calculate_total` has too many responsibilities. 
Consider extracting the tax calculation into a separate function:

def calculate_tax(subtotal, tax_rate):
    return subtotal * tax_rate

def calculate_total(subtotal, tax_rate, shipping):
    tax = calculate_tax(subtotal, tax_rate)
    return subtotal + tax + shipping
"""

2. Explain the Why

// Bad comment
// Use StringBuilder here

// Good comment
// Consider using StringBuilder instead of string concatenation in the loop.
// String concatenation creates a new string object on each iteration,
// while StringBuilder is more memory-efficient for this use case.
// 
// Example:
// var builder = new StringBuilder();
// foreach (var item in items) {
//     builder.AppendLine(item.ToString());
// }
// return builder.ToString();

3. Distinguish Between Must-Fix and Suggestions

// Critical issue (must fix)
// CRITICAL: This SQL query is vulnerable to SQL injection.
// Use prepared statements instead:
// 
// String query = "SELECT * FROM users WHERE id = ?";
// PreparedStatement stmt = conn.prepareStatement(query);
// stmt.setInt(1, userId);

// Suggestion (nice to have)
// SUGGESTION: Consider extracting this logic into a separate method
// to improve testability and readability. Not blocking but would
// improve maintainability.

4. Be Respectful and Constructive

# Bad
This is stupid. Who wrote this garbage?

# Good
This approach might cause issues with concurrent access. 
Have you considered using a thread-safe collection here?
The ConcurrentHashMap class would be a good fit for this use case.

# Bad
Obviously, this is wrong.

# Good
I think there might be an issue with this logic when the list is empty.
Should we add a check at the beginning of the function?

# Bad  
You don't know what you're doing.

# Good
I notice this pattern is different from our standard approach.
Let's discuss the trade-offs. Is there a specific reason for this choice?

5. Use the Right Tone

# Examples of constructive feedback tone

# Asking Questions
"""
How does this handle the case where user is None?
Should we add a null check or is it guaranteed to exist?
"""

# Making Suggestions
"""
This works, but we could make it more readable by using a list comprehension:

# Current
results = []
for item in items:
    if item.active:
        results.append(item.name)

# Suggested
results = [item.name for item in items if item.active]
"""

# Showing Appreciation
"""
Great job handling all the edge cases here! The error messages are
particularly helpful for debugging.
"""

# Learning Together
"""
Interesting approach! I haven't seen this pattern before.
Could you explain the benefits over the traditional approach?
I'd like to learn more about this.
"""

# Sharing Knowledge
"""
TIL (Today I Learned): Python 3.9 added the removeprefix() method
which makes this code simpler:

# Instead of
if text.startswith(prefix):
    text = text[len(prefix):]

# We can use
text = text.removeprefix(prefix)
"""

Code Review Comment Categories

// TypeScript - Structured comment examples

interface ReviewComment {
  type: CommentType;
  severity: Severity;
  message: string;
  suggestion?: string;
  line?: number;
}

enum CommentType {
  BUG = 'bug',
  PERFORMANCE = 'performance',
  SECURITY = 'security',
  STYLE = 'style',
  DESIGN = 'design',
  QUESTION = 'question',
  PRAISE = 'praise',
  SUGGESTION = 'suggestion'
}

enum Severity {
  CRITICAL = 'critical',    // Must fix before merging
  MAJOR = 'major',          // Should fix before merging
  MINOR = 'minor',          // Nice to fix
  NITPICK = 'nitpick'       // Optional
}

const exampleComments: ReviewComment[] = [
  {
    type: CommentType.BUG,
    severity: Severity.CRITICAL,
    message: 'Race condition in async operations',
    suggestion: 'Add mutex lock or use Promise.all()',
    line: 42
  },
  {
    type: CommentType.SECURITY,
    severity: Severity.CRITICAL,
    message: 'Potential XSS vulnerability - user input not sanitized',
    suggestion: 'Use DOMPurify.sanitize() before inserting into DOM',
    line: 156
  },
  {
    type: CommentType.PERFORMANCE,
    severity: Severity.MAJOR,
    message: 'O(n²) complexity - this will be slow with large datasets',
    suggestion: 'Consider using a Map for O(1) lookup',
    line: 89
  },
  {
    type: CommentType.DESIGN,
    severity: Severity.MAJOR,
    message: 'This class has too many responsibilities (SRP violation)',
    suggestion: 'Consider splitting into UserValidator and UserPersistence',
    line: 200
  },
  {
    type: CommentType.STYLE,
    severity: Severity.MINOR,
    message: 'Variable name doesn\'t follow camelCase convention',
    suggestion: 'Rename user_data to userData',
    line: 33
  },
  {
    type: CommentType.QUESTION,
    severity: Severity.MINOR,
    message: 'Why are we using setTimeout here? Is there a race condition?',
    line: 78
  },
  {
    type: CommentType.PRAISE,
    severity: Severity.NITPICK,
    message: 'Excellent error handling! Very clear error messages.',
    line: 101
  },
  {
    type: CommentType.SUGGESTION,
    severity: Severity.NITPICK,
    message: 'Consider using optional chaining here for cleaner code',
    suggestion: 'user?.address?.street instead of nested if checks',
    line: 67
  }
];

// Categorizing comments for the review summary
function categorizeComments(comments: ReviewComment[]): Map<Severity, ReviewComment[]> {
  const categorized = new Map<Severity, ReviewComment[]>();
  
  for (const comment of comments) {
    const existing = categorized.get(comment.severity) || [];
    existing.push(comment);
    categorized.set(comment.severity, existing);
  }
  
  return categorized;
}

// Generating review summary
function generateReviewSummary(comments: ReviewComment[]): string {
  const categorized = categorizeComments(comments);
  
  let summary = '## Review Summary\n\n';
  
  if (categorized.has(Severity.CRITICAL)) {
    summary += `⛔ ${categorized.get(Severity.CRITICAL)!.length} critical issues\n`;
  }
  if (categorized.has(Severity.MAJOR)) {
    summary += `⚠️ ${categorized.get(Severity.MAJOR)!.length} major issues\n`;
  }
  if (categorized.has(Severity.MINOR)) {
    summary += `ℹ️ ${categorized.get(Severity.MINOR)!.length} minor suggestions\n`;
  }
  if (categorized.has(Severity.NITPICK)) {
    summary += `💭 ${categorized.get(Severity.NITPICK)!.length} nitpicks\n`;
  }
  
  return summary;
}

Receiving Feedback

How to Handle Code Review Feedback

1. Don’t Take It Personally

Code review is about the code, not about you as a developer. Feedback is an opportunity to learn and improve.

2. Ask Questions

# Good responses to feedback

Reviewer: "This could cause a memory leak"
You: "Thanks for catching that! Can you explain how the memory leak would occur? 
     I want to make sure I understand so I don't make this mistake again."

Reviewer: "Consider using Strategy pattern here"
You: "I'm not familiar with Strategy pattern in this context. 
     Could you point me to an example or explain how it would work here?"

Reviewer: "This violates SOLID principles"
You: "Which principle specifically? I want to understand better 
     so I can refactor appropriately."

3. Explain Your Reasoning

Reviewer: "Why did you choose this approach?"
You: "Good question! I chose this approach because:
     1. It maintains backward compatibility with existing API
     2. Performance tests showed 40% improvement over alternative
     3. It matches the pattern we use in the OrderService module
     
     However, I'm open to alternatives if you have concerns."

4. Be Open to Learning

Reviewer: "Have you considered using async/await instead of promises?"
You: "I haven't used async/await much. You're right that it would be cleaner here.
     Let me refactor this. Could you recommend any good resources on async/await
     best practices?"

5. Push Back Politely When Needed

Reviewer: "Rewrite this entire module using functional programming"
You: "I appreciate the suggestion, but I have concerns:
     1. Timeline: This would add 2+ days and we're on a tight deadline
     2. Consistency: Rest of codebase uses OOP approach
     3. Team: Not everyone is comfortable with FP yet
     
     Could we discuss this in the next architecture meeting instead?
     For now, I'll address the specific issues you mentioned."

Common Code Review Pitfalls

For Reviewers

1. Nitpicking on Style

# Bad: Focusing too much on style
"""
Comment: "Add space after comma"
Comment: "Use single quotes instead of double quotes"
Comment: "This line is 81 characters, limit is 80"
"""

# Good: Use automated tools
"""
Add to .pre-commit-config.yaml:
- id: black
- id: flake8
- id: isort

Then focus review on logic, design, and business requirements.
"""

2. Reviewing Too Quickly

// Bad review: Rushed, surface-level
// "LGTM" (Looks Good To Me)

// Good review: Thorough, thoughtful
/*
Overall structure looks good. A few concerns:

1. Line 45: Should we handle the case where user is null?
   The subsequent code assumes it exists.

2. Lines 67-89: This error handling could be extracted to a 
   separate method for reusability.

3. Line 103: Consider caching this database call - it's in a loop
   and could cause N+1 query issues.

4. Tests: Could we add a test for the edge case where the list is empty?

5. Positive note: Excellent documentation on the complex algorithm!
*/

3. Not Providing Context

// Bad comment
// Change this

// Good comment
// This recursive approach could cause stack overflow with deep nesting.
// Consider using an iterative approach with an explicit stack:
//
// Stack<Node> stack = new Stack<>();
// stack.push(root);
// while (!stack.isEmpty()) {
//     Node node = stack.pop();
//     // Process node
//     if (node.left != null) stack.push(node.left);
//     if (node.right != null) stack.push(node.right);
// }

4. Being Inconsistent

// Bad: Inconsistent feedback across the codebase
// In file1.ts: "Don't use any type"
// In file2.ts: [No comment on any type usage]

// Good: Consistent standards
// Create team coding guidelines and apply them uniformly
// Document decisions in ADRs (Architecture Decision Records)

For Authors

1. Submitting Too Much at Once

# Bad: Massive PR
$ git diff main --stat
 43 files changed, 3421 insertions(+), 1876 deletions(-)

# Good: Focused PR
$ git diff main --stat
 5 files changed, 234 insertions(+), 89 deletions(-)

2. Not Responding to Feedback

# Bad
[No response to reviewer comments, just push new commits]

# Good
Reviewer: "This could cause issues with null values"
You: "Good catch! Fixed in commit abc123. Added null check and test case."

3. Being Defensive

# Bad
Reviewer: "This approach might have performance issues"
You: "No, you're wrong. This is the best way to do it."

# Good
Reviewer: "This approach might have performance issues"
You: "Thanks for the concern. I ran benchmarks comparing this with 
     the alternative approach (results attached). This performs 
     better for our typical dataset size (< 1000 items).
     For larger datasets, you're right that we'd need a different approach.
     Should we add a TODO comment about this limitation?"

Code Review Tools and Automation

Popular Code Review Tools

GitHub Pull Requests

# .github/pull_request_template.md
## Description
[Describe what this PR does]

## Type of Change
- [ ] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update

## Testing
- [ ] Unit tests pass
- [ ] Integration tests pass
- [ ] Manual testing completed

## Checklist
- [ ] Code follows style guidelines
- [ ] Self-review completed
- [ ] Comments added for complex code
- [ ] Documentation updated
- [ ] No new warnings generated

GitLab Merge Requests

# .gitlab/merge_request_templates/default.md
## What does this MR do?

## Related issues
Closes #

## Checklist
- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] Changelog entry added
- [ ] Security implications considered

Automated Code Review

Linting and Formatting

# .github/workflows/code-quality.yml
name: Code Quality

on: [pull_request]

jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      
      - name: Run ESLint
        run: npm run lint
      
      - name: Run Prettier
        run: npm run format:check
      
      - name: Run tests
        run: npm test
      
      - name: Check coverage
        run: npm run test:coverage
        
      - name: Comment PR
        uses: actions/github-script@v5
        if: failure()
        with:
          script: |
            github.rest.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: '❌ Code quality checks failed. Please fix the issues and push again.'
            })

Security Scanning

# .github/workflows/security.yml
name: Security Scan

on: [pull_request]

jobs:
  security:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      
      - name: Run Snyk
        uses: snyk/actions/node@master
        env:
          SNYK_TOKEN: $
      
      - name: Run CodeQL
        uses: github/codeql-action/analyze@v2
      
      - name: Run dependency check
        run: npm audit

Code Coverage

// jest.config.js
module.exports = {
  collectCoverage: true,
  coverageThreshold: {
    global: {
      branches: 80,
      functions: 80,
      lines: 80,
      statements: 80
    }
  },
  coverageReporters: ['text', 'lcov', 'html']
};

Code Review Bots

Example: PR Review Bot

# Python - Simple PR review bot
from github import Github
import re

class PRReviewBot:
    def __init__(self, github_token, repo_name):
        self.github = Github(github_token)
        self.repo = self.github.get_repo(repo_name)
    
    def review_pr(self, pr_number):
        pr = self.repo.get_pull(pr_number)
        comments = []
        
        # Check PR size
        if pr.additions + pr.deletions > 500:
            comments.append({
                'body': '⚠️ This PR is quite large (>500 lines). '
                       'Consider breaking it into smaller PRs for easier review.',
                'path': None,
                'line': None
            })
        
        # Check PR description
        if not pr.body or len(pr.body) < 50:
            comments.append({
                'body': '📝 Please add a more detailed description of the changes.',
                'path': None,
                'line': None
            })
        
        # Check for tests
        files = pr.get_files()
        has_test_changes = any('test' in f.filename.lower() for f in files)
        has_code_changes = any(
            f.filename.endswith(('.py', '.js', '.java', '.cs'))
            and 'test' not in f.filename.lower()
            for f in files
        )
        
        if has_code_changes and not has_test_changes:
            comments.append({
                'body': '🧪 No test files modified. Did you add tests for your changes?',
                'path': None,
                'line': None
            })
        
        # Check for console.log / print statements
        for file in files:
            if file.patch:
                if 'console.log' in file.patch:
                    comments.append({
                        'body': '🐛 Found console.log statement. Consider using a proper logging library.',
                        'path': file.filename,
                        'line': None
                    })
                
                if re.search(r'^\+.*print\(', file.patch, re.MULTILINE):
                    comments.append({
                        'body': '🐛 Found print statement. Use logging module instead.',
                        'path': file.filename,
                        'line': None
                    })
        
        # Post comments
        for comment in comments:
            if comment['path']:
                # File-specific comment
                pr.create_review_comment(
                    body=comment['body'],
                    commit=pr.get_commits()[0],
                    path=comment['path'],
                    line=1
                )
            else:
                # General PR comment
                pr.create_issue_comment(comment['body'])
        
        # Overall review status
        if len(comments) == 0:
            pr.create_review(
                body='✅ Automated checks passed! Ready for human review.',
                event='COMMENT'
            )
        else:
            pr.create_review(
                body=f'⚠️ Found {len(comments)} potential issues. Please review.',
                event='COMMENT'
            )

Code Review Metrics

Measuring Code Review Effectiveness

# Python - Code review metrics tracker
from dataclasses import dataclass
from datetime import datetime, timedelta
from typing import List
import statistics

@dataclass
class PullRequest:
    id: int
    created_at: datetime
    merged_at: datetime
    lines_changed: int
    review_comments: int
    reviewers: List[str]
    author: str

class CodeReviewMetrics:
    def __init__(self, pull_requests: List[PullRequest]):
        self.prs = pull_requests
    
    def average_review_time(self) -> timedelta:
        """Time from PR creation to merge"""
        times = [
            pr.merged_at - pr.created_at
            for pr in self.prs
            if pr.merged_at
        ]
        if not times:
            return timedelta(0)
        return sum(times, timedelta(0)) / len(times)
    
    def review_thoroughness(self) -> float:
        """Comments per 100 lines of code"""
        total_comments = sum(pr.review_comments for pr in self.prs)
        total_lines = sum(pr.lines_changed for pr in self.prs)
        if total_lines == 0:
            return 0
        return (total_comments / total_lines) * 100
    
    def reviewer_participation(self) -> dict:
        """Number of reviews per reviewer"""
        participation = {}
        for pr in self.prs:
            for reviewer in pr.reviewers:
                participation[reviewer] = participation.get(reviewer, 0) + 1
        return participation
    
    def pr_size_distribution(self) -> dict:
        """Distribution of PR sizes"""
        sizes = {'small': 0, 'medium': 0, 'large': 0, 'huge': 0}
        for pr in self.prs:
            if pr.lines_changed < 200:
                sizes['small'] += 1
            elif pr.lines_changed < 500:
                sizes['medium'] += 1
            elif pr.lines_changed < 1000:
                sizes['large'] += 1
            else:
                sizes['huge'] += 1
        return sizes
    
    def review_coverage(self) -> float:
        """Percentage of PRs with at least one reviewer"""
        reviewed = sum(1 for pr in self.prs if len(pr.reviewers) > 0)
        return (reviewed / len(self.prs)) * 100 if self.prs else 0
    
    def generate_report(self) -> str:
        report = "Code Review Metrics Report\n"
        report += "=" * 50 + "\n\n"
        
        avg_time = self.average_review_time()
        report += f"Average Review Time: {avg_time.total_seconds() / 3600:.1f} hours\n"
        
        thoroughness = self.review_thoroughness()
        report += f"Review Thoroughness: {thoroughness:.2f} comments per 100 lines\n"
        
        coverage = self.review_coverage()
        report += f"Review Coverage: {coverage:.1f}%\n\n"
        
        report += "PR Size Distribution:\n"
        sizes = self.pr_size_distribution()
        for size, count in sizes.items():
            report += f"  {size}: {count} PRs\n"
        
        report += "\nTop Reviewers:\n"
        participation = self.reviewer_participation()
        sorted_reviewers = sorted(
            participation.items(),
            key=lambda x: x[1],
            reverse=True
        )[:5]
        for reviewer, count in sorted_reviewers:
            report += f"  {reviewer}: {count} reviews\n"
        
        return report

# Usage example
prs = [
    PullRequest(
        id=1,
        created_at=datetime(2024, 1, 1, 10, 0),
        merged_at=datetime(2024, 1, 1, 14, 30),
        lines_changed=150,
        review_comments=8,
        reviewers=['alice', 'bob'],
        author='charlie'
    ),
    PullRequest(
        id=2,
        created_at=datetime(2024, 1, 2, 9, 0),
        merged_at=datetime(2024, 1, 3, 16, 0),
        lines_changed=450,
        review_comments=15,
        reviewers=['alice', 'dave'],
        author='bob'
    ),
]

metrics = CodeReviewMetrics(prs)
print(metrics.generate_report())

Key Metrics to Track

Process Metrics

• Average time to first review
• Average time to merge
• Number of review cycles
• PR size (lines of code)

Quality Metrics

• Number of comments per PR
• Comment resolution rate
• Post-merge bug rate
• Test coverage change

Participation Metrics

• Reviews per developer
• Review distribution
• Response time
• Approval rate

Team Code Review Guidelines

Creating a Code Review Culture

1. Document Standards

# team-guidelines.md

## Code Review Standards

### For Authors
- PRs should be < 400 lines
- Include tests for new features
- Update documentation
- Self-review before submitting
- Respond to feedback within 24 hours

### For Reviewers
- Review within 24 hours
- Be constructive and respectful
- Explain the "why" behind feedback
- Approve if no blocking issues
- Use labels: critical, major, minor, nitpick

### For Everyone
- Code review is a learning opportunity
- Focus on code, not person
- Ask questions instead of making demands
- Celebrate good code
- Keep discussions professional

2. Review Checklist Template

## Code Review Checklist

### Functionality
- [ ] Code does what it's supposed to do
- [ ] Edge cases are handled
- [ ] Error handling is appropriate
- [ ] No obvious bugs

### Design
- [ ] Follows SOLID principles
- [ ] Appropriate design patterns used
- [ ] No over-engineering
- [ ] Proper separation of concerns

### Code Quality
- [ ] Code is readable and maintainable
- [ ] Names are descriptive
- [ ] No code duplication
- [ ] Complexity is reasonable
- [ ] Follows team conventions

### Testing
- [ ] Unit tests added/updated
- [ ] Tests cover edge cases
- [ ] Tests are meaningful
- [ ] Test code quality is good

### Documentation
- [ ] Code is self-documenting
- [ ] Complex logic is explained
- [ ] API documentation updated
- [ ] README updated if needed

### Security
- [ ] Input is validated
- [ ] No injection vulnerabilities
- [ ] Auth/authz is correct
- [ ] No secrets in code

### Performance
- [ ] No obvious performance issues
- [ ] Efficient algorithms used
- [ ] Resources properly managed
- [ ] Scalability considered

Conclusion

Code reviews are essential for maintaining code quality, sharing knowledge, and building strong development teams. The key to effective code reviews is:

• Keep PRs small and focused
• Review promptly and thoroughly
• Be constructive and respectful
• Focus on learning and improvement
• Use automation for routine checks
• Measure and improve the process

Remember that code review is not just about finding bugs – it’s about creating better code, better developers, and better teams.

References

1. Atwood, Jeff. “Code Reviews: Just Do It.” Coding Horror Blog, 2006.
2. McConnell, Steve. “Code Complete: A Practical Handbook of Software Construction.” Microsoft Press, 2004.
3. Fogel, Karl. “Producing Open Source Software.” O’Reilly Media, 2005.
4. Bacchelli, Alberto, and Christian Bird. “Expectations, outcomes, and challenges of modern code review.” ICSE 2013.
5. Rigby, Peter C., and Christian Bird. “Convergent software peer review practices.” FSE 2013.
6. Google Engineering Practices: “How to do a code review.” https://google.github.io/eng-practices/review/
7. Atlassian. “Code Review Best Practices.” https://www.atlassian.com/agile/software-development/code-reviews
8. Microsoft. “Code Review Guidelines.” Azure DevOps Documentation, 2022.
9. Perforce. “9 Best Practices for Code Review.” 2021.
10. Smart Bear. “Best Practices for Peer Code Review.” 2020.