Code review skill

Code Review Skill

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

For large diffs, pipe the diff through scripts/pr-analyzer.py (git diff main...HEAD | python scripts/pr-analyzer.py) to triage complexity and get a suggested review approach before reading.

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

1. Architecture & Design - Does the solution fit the problem?
   • For significant changes, consult Architecture Review Guide
   • Check: SOLID principles, coupling/cohesion, anti-patterns
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!

Severity levels: 🔴 / 🟡 / 🟢 are the three severity tiers used as the standard across all guides in this skill — 🔴 blocks the merge, 🟡 should be addressed, 🟢 is optional. The remaining markers (💡 / 📚 / 🎉) are non-blocking annotations.

Language-Specific Guides

根据审查的代码语言，查阅对应的详细指南：

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, 响应性系统, Props/Emits, Watchers, Composables
Angular 17+	Angular Guide	Signals, Standalone, RxJS, Zoneless, 模板优化, 测试, 路由守卫, HttpInterceptor
Rust	Rust Guide	所有权/借用, Unsafe 审查, 异步代码, 取消安全性, 错误处理
TypeScript	TypeScript Guide	类型安全, async/await, 不可变性, 测试, 模块解析, TS 5.x
Python	Python Guide	可变默认参数, 异常处理, 类属性
Django / DRF	Django Guide	安全审查, N+1 查询, Serializer 反模式, ViewSet, 异步视图
FastAPI	FastAPI Guide	Depends, Pydantic v2 validation, async correctness, sessions/N+1, auth vs authorization, test-driven verification
Java	Java Guide	Java 17/21 新特性, Spring Boot 3, 虚拟线程, Stream/Optional
Java 8 / Legacy	Java 8 Guide	Java 8, Spring Boot 2, javax.*, Stream/Optional, java.time, CompletableFuture
PHP	PHP Guide	PHP 8.x type system, PDO, security review, Composer, PHPUnit/PHPStan
C# / .NET	C# Guide	C# 12 特性, 异步编程, EF Core 性能, ASP.NET Core, LINQ
Go	Go Guide	错误处理, goroutine/channel, context, 接口设计
Kotlin / Android	Kotlin Guide	协程, Flow, Jetpack Compose, 空安全, 内存泄漏, 架构模式
Swift / SwiftUI	Swift Guide	Optionals, Swift Concurrency, Sendable/actors, SwiftUI property wrappers, value vs reference types, API design
NestJS	NestJS Guide	依赖注入, 分层架构, DTO 验证, Guard/Interceptor, 循环依赖
Svelte / SvelteKit	Svelte Guide	Runes, Load 函数, Form Actions, Store 迁移, SSR/CSR 边界
C	C Guide	指针/缓冲区, 内存安全, UB, 安全编码, 可移植性, 测试
C++	C++ Guide	RAII, 智能指针, C++20/23, constexpr, 测试
Zig	Zig Guide	Allocators, error unions, defer/errdefer, comptime, C interop
CSS/Less/Sass	CSS Guide	变量规范, !important, 性能优化, 响应式, 兼容性
Qt	Qt Guide	对象模型, 信号/槽, Model/View, QML, Qt6 迁移, 测试

Cross-Cutting Guides

Language-agnostic patterns applicable to all code reviews:

Topic	Reference File	Key Topics
Architecture Review	Architecture Review Guide	SOLID, anti-patterns, coupling/cohesion, dependency direction
Performance Review	Performance Review Guide	Web Vitals, N+1, algorithm complexity, memory leaks, caching
Security Review	Security Review Guide	SQLi, XSS, CSRF, SSRF, IDOR, 命令注入, 跨语言示例
Universal Quality	Universal Quality Guide	Reuse audit, parameter sprawl, leaky abstractions, nested conditionals, stringly-typed code, TOCTOU, no-op updates, redundant state
Common Bugs	Common Bugs Checklist	Language-specific bug patterns, common pitfalls
SQL Injection Prevention	SQL Injection Guide	Parameterized queries, ORM safety, 6 languages, dynamic identifiers, detection
XSS Prevention	XSS Prevention Guide	Output encoding, CSP, 5 frameworks, input validation vs encoding, detection
N+1 Queries	N+1 Queries Guide	Eager loading, batch fetching, DataLoader, 5 languages, detection
Error Handling	Error Handling Guide	Fail fast, error hierarchy, 7 languages, anti-patterns, logging
Async & Concurrency	Concurrency Guide	Goroutines, async/await, actors, structured concurrency, 7 languages
Review Best Practices	Code Review Best Practices	Communication, reviewer mindset, giving feedback, severity labels

Additional Resources

• PR Review Template - PR 审查评论模板
• Review Checklist - 快速参考清单