anupjha-blog
GitHubGitHub RepositoryXTwitterLinkedInLinkedIn

Code Review Best Practices - Organized Guide

anupjha

anupjha

@imanupjha

1. PREPARATION & SETUP

Making Reviews Manageable

  • Keep pull requests small - Break down changes into digestible chunks that are easy to understand and review
    • Example: Instead of submitting a 500-line PR with feature addition + refactoring + bug fixes, create separate PRs for each component
  • Write clear pull request descriptions - Explain the "what" and "why" of changes with proper context
    • Example: "Adding user authentication (relates to JIRA-123). This implements OAuth2 login because we need to support SSO. Screenshot attached showing the new login flow."
  • Ask for review only after tests & builds pass - Ensure code is stable before requesting reviewer time
    • Example: Wait for CI/CD pipeline to go green before clicking "Request Review" - this prevents reviewers from finding basic syntax errors
  • Link relevant tickets and attach screenshots - Provide context that helps reviewers understand the change
    • Example: Include before/after screenshots for UI changes, or link to the original bug report for context

Setting Up Review Infrastructure

  • Use a code review checklist - Ensure consistency covering design, readability, security, and testing
    • Example: Checklist items like "Are edge cases tested?", "Is error handling implemented?", "Does this follow our naming conventions?"
  • Automate easy parts - Use tests, linters, and static analysis to catch errors and style issues
    • Example: Configure ESLint, Prettier, or SonarQube to automatically flag style violations and common bugs before human review
  • Use review tools effectively - Leverage threaded comments, suggested edits, and templates
    • Example: Use GitHub's "suggestion" feature to propose code changes directly, or create PR templates that prompt for security considerations

2. REVIEWER SELECTION & RESPONSIBILITY

Choosing the Right Reviewers

  • Choose the correct reviewer for each change - Assign code owners or domain experts who can catch domain-specific issues
    • Example: For database migration changes, assign someone from the data engineering team rather than a frontend developer
  • Clarify reviewer responsibilities - When assigning multiple reviewers, ensure each understands their role
    • Example: "Alice: focus on security aspects; Bob: verify business logic; Carol: check API design"
  • Encourage all team members to participate - Rotate reviewers to spread knowledge and avoid burnout
    • Example: Junior developers review senior code (great learning), senior developers review junior code (mentorship), peers cross-review

3. CODE QUALITY & CORRECTNESS

Basic Code Hygiene

  • Watch for duplicate & dead code - Remove unused code and abstract logic to avoid duplication
    • Example: Identify functions that appear in multiple files and extract them into a shared utility module
  • Verify sufficient test coverage - Good tests prevent regressions and demonstrate how code should work
    • Example: If adding a new payment processing function, ensure tests cover successful payment, failed payment, timeout scenarios, and invalid inputs

Logic & Implementation Review

  • Watch out for potential bugs & logic mistakes - Think about race conditions or extreme inputs that tests might miss
    • Example: Check if concurrent requests could corrupt shared state, or if the function handles empty arrays/null values properly
  • Compare implementation with requirements - Ensure it handles acceptance criteria, edge cases, and error conditions
    • Example: If the ticket says "users should see an error message when upload fails," verify this behavior exists for all failure types
  • Ensure code handles errors gracefully - Functions must deal with null inputs or external call failures without crashing
    • Example: Instead of user.name.toUpperCase(), use user?.name?.toUpperCase() ?? 'Unknown' to prevent crashes

Performance & Scalability

  • Consider performance at scale - Look for things that might cause slowdowns in critical paths
    • Example: Avoid nested loops like users.forEach(user => orders.forEach(order => ...)) when dealing with large datasets - use a hash map instead
  • Review with the bigger picture in mind - Consider cross-cutting concerns like performance, concurrency, and backward compatibility
    • Example: A new API field should be optional to maintain backward compatibility with mobile apps running older versions

4. CODE STANDARDS & CONSISTENCY

Maintaining Standards

  • Enforce coding standards for consistency - Suggest refactoring if logic is hard to follow
    • Example: If the team uses async/await, point out callback-based code that should be updated to match the standard
  • Focus on code correctness & clarity, not personal style - Let stylistic preferences pass if not covered by guidelines
    • Example: Don't argue about single vs. double quotes if the linter doesn't enforce it; focus on whether the logic is correct

Documentation & Clarity

  • Consider whether documentation needs updates - API changes may require updates to docs or README files
    • Example: If a function signature changes from getUser(id) to getUser(id, includeDeleted), update the API documentation and inline comments

5. REVIEW PROCESS & TIMING

Managing Review Flow

  • Review quickly, but don't rush - The goal is to improve code health, not just quick approvals
    • Example: Aim to provide initial feedback within 4-8 hours, but take the time needed to understand complex logic
  • Keep reviews short - Hard to focus after 100+ lines of code
    • Example: If reviewing a 500-line change, review one module at a time with breaks, or ask the author to split it up
  • Get early feedback on big features - Catch issues early and make reviews more manageable
    • Example: For a 3-week feature, request a design review after day 2, then incremental PR reviews rather than one massive final review

Review Approach

  • Review in layers: design then details - Catch both major and minor issues efficiently
    • Example: First pass: check architecture and design patterns. Second pass: look at variable names, error handling, and edge cases
  • Read code carefully and run it locally if necessary - You can't review effectively without understanding what it does
    • Example: For complex state management changes, checkout the branch and click through the UI to understand the behavior
  • Keep feedback within scope - Log out-of-scope issues separately
    • Example: If reviewing authentication changes and you notice unrelated logging issues, create a separate ticket rather than blocking the PR

6. METRICS & CONTINUOUS IMPROVEMENT

Data-Driven Improvements

  • Use review metrics to find bottlenecks - Measure review time, bug rates, and pull request size
    • Example: If average review time is 3 days, investigate whether PRs are too large, reviewers are overloaded, or descriptions are unclear
  • Adjust practices to fit your team's needs - Keep experimenting until you find your ideal flow
    • Example: A startup might use single-reviewer approval for speed, while a banking app requires two reviewers for critical paths

Tool Enhancement

  • Use AI tools to summarize changes or find issues - As a helper, not a replacement for human reviews
    • Example: Use GitHub Copilot or similar tools to get quick summaries of large changes, then dive deeper into critical sections

7. COMMUNICATION & COLLABORATION

Effective Feedback

  • Explain the "why" behind feedback - Help others learn and avoid repeating issues
    • Example: Instead of "Don't use var," say "Use const/let instead of var because var has function-scope which can cause unexpected hoisting bugs"
  • Suggest solutions when pointing out problems - Reviews are most valuable when they teach
    • Example: "This function is too complex (20+ lines, multiple responsibilities). Consider extracting the validation logic into a separate validateInput() function"
  • Mention which comments are essential vs. optional - Help authors prioritize
    • Example: "🔴 BLOCKER: Security issue - SQL injection vulnerability. 🟡 OPTIONAL: Consider renaming tmp to tempUser for clarity"

Positive Team Culture

  • Treat code review as team effort, not a fight - Focus on making the product better
    • Example: Use "we" language: "We could improve this by..." rather than "You should fix..."
  • Point out what's done well - Balance criticism with appreciation
    • Example: "Great job adding comprehensive error messages! This will make debugging much easier."
  • Be open to discussion when opinions differ - Ask for reasoning and listen before insisting
    • Example: If someone uses a pattern you disagree with, ask "What was your reasoning for this approach?" before suggesting alternatives
  • Respond to feedback with curiosity, not defensiveness - Treat comments as learning opportunities
    • Example: When receiving feedback, respond with "Thanks! I didn't consider that edge case. I'll add a test for it."

Clarification & Understanding

  • Ask clarifying questions when unclear - Prevent misunderstandings or reveal missing requirements
    • Example: "I see you're caching this data. How do we handle cache invalidation when the user updates their profile?"

8. SECURITY & RISK MANAGEMENT

Security Focus

  • Always think about security - Secure code protects users and the business
    • Example: Check for SQL injection vulnerabilities, XSS in user inputs, exposed API keys, or sensitive data in logs
  • Be cautious of weak data validation, exposed data, or improper error handling
    • Example: Verify that error messages don't expose sensitive information like database structure or internal file paths

Approval Standards

  • Set clear guidelines for approval - Define review requirements for critical changes
    • Example: "All changes to payment processing require approval from 2 senior engineers + security team review"
  • Have another set of eyes review every change - Even small changes benefit from peer review
    • Example: Even a one-line configuration change should be reviewed to catch typos or misconfigurations

9. KNOWLEDGE SHARING & LEARNING

Team Growth

  • Use reviews as an opportunity to share knowledge - Share tips and best practices
    • Example: "FYI: You can use the built-in Array.groupBy() method instead of writing this custom grouping logic"
  • Focus on teaching, not just criticizing
    • Example: Link to relevant documentation or internal wiki pages when suggesting improvements

10. CONFLICT RESOLUTION

Handling Disagreements

  • Involve a neutral third party for critical disagreements - Ask a tech lead or architect
    • Example: If two developers disagree on whether to use REST vs GraphQL for a new API, escalate to the architecture team
  • Create follow-up tasks for out-of-scope problems
    • Example: "The performance concern you raised is valid but outside this PR's scope. I've created JIRA-456 to address it separately"

11. CULTURAL GUIDELINES

Review Philosophy

  • Don't use code reviews for performance evaluations - Reviews exist to improve code, not measure people
    • Example: Mistakes found in review should be learning opportunities, not ammunition for annual reviews
  • Create psychological safety - When engineers feel safe, they write better code and review honestly
    • Example: Encourage asking questions without judgment: "I'm not familiar with this pattern - can you explain the benefits?"
xs