SuperAPI AI Directory

code-review-excellence

Testing & quality

View repository

Updated 10-13 21:39

About this skill

Code Review Excellence

Transform code reviews from gatekeeping to knowledge sharing through constructive feedback, systematic analysis, and collaborative improvement.

When to Use This Skill

  • Reviewing pull requests and code changes
  • Establishing code review standards for teams
  • Mentoring junior developers through reviews
  • Conducting architecture reviews
  • Creating review checklists and guidelines
  • Improving team collaboration
  • Reducing code review cycle time
  • Maintaining code quality standards

Core Principles

1. The Review Mindset

Goals of Code Review:

  • Catch bugs and edge cases
  • Ensure code maintainability
  • Share knowledge across team
  • Enforce coding standards
  • Improve design and architecture
  • Build team culture

Not the Goals:

  • Show off knowledge
  • Nitpick formatting (use linters)
  • Block progress unnecessarily
  • Rewrite to your preference

2. Effective Feedback

Good Feedback is:

  • Specific and actionable
  • Educational, not judgmental
  • Focused on the code, not the person
  • Balanced (praise good work too)
  • Prioritized (critical vs nice-to-have)
❌ Bad: "This is wrong."
✅ Good: "This could cause a race condition when multiple users
         access simultaneously. Consider using a mutex here."

❌ Bad: "Why didn't you use X pattern?"
✅ Good: "Have you considered the Repository pattern? It would
         make this easier to test. Here's an example: [link]"

❌ Bad: "Rename this variable."
✅ Good: "[nit] Consider `userCount` instead of `uc` for
         clarity. Not blocking if you prefer to keep it."

3. Review Scope

What to Review:

  • Logic correctness and edge cases
  • Security vulnerabilities
  • Performance implications
  • Test coverage and quality
  • Error handling
  • Documentation and comments
  • API design and naming
  • Architectural fit

What Not to Review Manually:

  • Code formatting (use Prettier, Black, etc.)
  • Import organization
  • Linting violations
  • Simple typos

Review Process

Phase 1: Context Gathering (2-3 minutes)

Before diving into code, understand:

  1. Read PR description and linked issue
  2. Check PR size (>400 lines? Ask to split)
  3. Review CI/CD status (tests passing?)
  4. Understand the business requirement
  5. Note any relevant architectural decisions

Phase 2: High-Level Review (5-10 minutes)

  1. Architecture & Design - Does the solution fit the problem?
  2. Performance Assessment - Are there performance concerns?
    • For performance-critical code, consult Performance Review Guide
    • Check: Algorithm complexity, N+1 queries, memory usage
  3. File Organization - Are new files in the right places?
  4. Testing Strategy - Are there tests covering edge cases?

Phase 3: Line-by-Line Review (10-20 minutes)

For each file, check:

  • Logic & Correctness - Edge cases, off-by-one, null checks, race conditions
  • Security - Input validation, injection risks, XSS, sensitive data
  • Performance - N+1 queries, unnecessary loops, memory leaks
  • Maintainability - Clear names, single responsibility, comments
  • Reuse - Before accepting new code, search for existing utilities/helpers that could replace it. Check adjacent files and shared modules for similar patterns. See Universal Quality Guide for anti-patterns like parameter sprawl, leaky abstractions, nested conditionals, stringly-typed code, TOCTOU, and no-op updates.

Phase 4: Summary & Decision (2-3 minutes)

  1. Summarize key concerns
  2. Highlight what you liked
  3. Make clear decision:
    • ✅ Approve
    • 💬 Comment (minor suggestions)
    • 🔄 Request Changes (must address)
  4. Offer to pair if complex

Review Techniques

Technique 1: The Checklist Method

Use checklists for consistent reviews. See Security Review Guide for comprehensive security checklist.

Technique 2: The Question Approach

Instead of stating problems, ask questions:

❌ "This will fail if the list is empty."
✅ "What happens if `items` is an empty array?"

❌ "You need error handling here."
✅ "How should this behave if the API call fails?"

Technique 3: Suggest, Don't Command

Use collaborative language:

❌ "You must change this to use async/await"
✅ "Suggestion: async/await might make this more readable. What do you think?"

❌ "Extract this into a function"
✅ "This logic appears in 3 places. Would it make sense to extract it?"

Technique 4: Differentiate Severity

Use labels to indicate priority:

  • 🔴 [blocking] - Must fix before merge
  • 🟡 [important] - Should fix, discuss if disagree
  • 🟢 [nit] - Nice to have, not blocking
  • 💡 [suggestion] - Alternative approach to consider
  • 📚 [learning] - Educational comment, no action needed
  • 🎉 [praise] - Good work, keep it up!

Language-Specific Guides

Based on the reviewed code language, consult the corresponding detailed guide:

Language/Framework Reference File Key Topics
React React Guide Hooks, useEffect, React 19 Actions, RSC, Suspense, TanStack Query v5
Vue 3 Vue Guide Composition API, reactivity system, Props/Emits, Watchers, Composables
Angular 17+ Angular Guide Signals, Standalone components, RxJS, Zoneless change detection, template optimization
Rust Rust Guide Ownership/borrowing, Unsafe review, asynchronous code, cancellation safety, error handling
TypeScript TypeScript Guide Type safety, async/await, immutability
Python Python Guide Mutable default arguments, exception handling, class attributes
Django / DRF Django Guide Security review, N+1 queries, Serializer anti-patterns, ViewSet, async views
FastAPI FastAPI Guide Depends, Pydantic v2 validation, async correctness, sessions/N+1, auth vs authorization, test-driven verification
Java Java Guide Java 17/21 new features, Spring Boot 3, virtual threads, Stream/Optional
PHP PHP Guide PHP 8.x type system, PDO, security review, Composer, PHPUnit/PHPStan
C# / .NET C# Guide C# 12 features, asynchronous programming, EF Core performance, ASP.NET Core, LINQ
Go Go Guide Error handling, goroutine/channel, context, interface design
Kotlin / Android Kotlin Guide Coroutines, Flow, Jetpack Compose, null safety, memory leaks, architecture patterns
NestJS NestJS Guide Dependency injection, layered architecture, DTO validation, Guard/Interceptor, circular dependencies
Svelte / SvelteKit Svelte Guide Runes, Load functions, Form Actions, Store migration, SSR/CSR boundaries
C C Guide Pointers/buffers, memory safety, UB, error handling
C++ C++ Guide RAII, lifetimes, Rule of 0/3/5, exception safety
CSS/Less/Sass CSS Guide Variable conventions, !important, performance optimization, responsiveness, compatibility
Qt Qt Guide Object model, signals/slots, memory management, thread safety, performance

Cross-Cutting Guides

Language-agnostic patterns applicable to all code reviews:

Topic Reference File Key Topics
Universal Quality Universal Quality Guide Reuse audit, parameter sprawl, leaky abstractions, nested conditionals, stringly-typed code, TOCTOU, no-op updates, redundant state

Additional Resources