Back to Details

code-review-playbook

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Playbook

代码评审指南

This skill provides a comprehensive framework for effective code reviews that improve code quality, share knowledge, and foster collaboration. Whether you're a reviewer giving feedback or an author preparing code for review, this playbook ensures reviews are thorough, consistent, and constructive.
本Skill为高效代码评审提供全面框架，可提升代码质量、促进知识共享并增强团队协作。无论你是给出反馈的评审者，还是准备代码接受评审的开发者，本指南都能确保评审过程全面、一致且富有建设性。

Overview

概述

  • Reviewing pull requests or merge requests
  • Preparing code for review (self-review)
  • Establishing code review standards for teams
  • Training new developers on review best practices
  • Resolving disagreements about code quality
  • Improving review processes and efficiency
  • 评审Pull Request（PR）或Merge Request（MR）
  • 为评审准备代码（自我评审）
  • 为团队建立代码评审标准
  • 培训新开发者掌握评审最佳实践
  • 解决关于代码质量的分歧
  • 提升评审流程与效率

Code Review Philosophy

代码评审理念

Purpose of Code Reviews

代码评审的目标

Code reviews serve multiple purposes:
  1. Quality Assurance: Catch bugs, logic errors, and edge cases
  2. Knowledge Sharing: Spread domain knowledge across the team
  3. Consistency: Ensure codebase follows conventions and patterns
  4. Mentorship: Help developers improve their skills
  5. Collective Ownership: Build shared responsibility for code
  6. Documentation: Create discussion history for future reference
代码评审具备多重目标：
  1. 质量保障：捕获bug、逻辑错误和边界情况
  2. 知识共享：在团队内传播领域知识
  3. 一致性：确保代码库遵循约定与模式
  4. 导师指导：帮助开发者提升技能
  5. 集体所有权：建立代码的共同责任意识
  6. 文档留存：为未来参考创建讨论历史

Principles

原则

Be Kind and Respectful:
  • Review the code, not the person
  • Assume positive intent
  • Praise good solutions
  • Frame feedback constructively
Be Specific and Actionable:
  • Point to specific lines of code
  • Explain why something should change
  • Suggest concrete improvements
  • Provide examples when helpful
Balance Speed with Thoroughness:
  • Aim for timely feedback (< 24 hours)
  • Don't rush critical reviews
  • Use automation for routine checks
  • Focus human review on logic and design
Distinguish Must-Fix from Nice-to-Have:
  • Use conventional comments to indicate severity
  • Block merges only for critical issues
  • Allow authors to defer minor improvements
  • Capture deferred work in follow-up tickets
友善且尊重他人：
  • 评审代码，而非针对个人
  • 假设对方出发点是好的
  • 认可优秀的解决方案
  • 以建设性的方式提出反馈
具体且可执行：
  • 指向具体的代码行
  • 解释为什么需要修改
  • 提出明确的改进建议
  • 必要时提供示例
平衡速度与全面性：
  • 争取及时给出反馈（<24小时）
  • 关键评审切勿仓促
  • 用自动化工具处理常规检查
  • 人工评审聚焦于逻辑与设计
区分必须修复与建议优化：
  • 使用标准化评论标注问题严重程度
  • 仅因关键问题阻止合并
  • 允许开发者延后处理次要改进
  • 将延后的工作记录在后续工单中

Conventional Comments

标准化评论规范

A standardized format for review comments that makes intent clear.
一种标准化的评审评论格式，可明确传达意图。

Format

格式

<label> [decorations]: <subject>

[discussion]
<label> [decorations]: <subject>

[discussion]

Labels

标签

LabelMeaningBlocks Merge?
praiseHighlight something positiveNo
nitpickMinor, optional suggestionNo
suggestionPropose an improvementNo
issueProblem that should be addressedUsually
questionRequest clarificationNo
thoughtIdea to considerNo
choreRoutine task (formatting, deps)No
noteInformational commentNo
todoFollow-up work neededMaybe
securitySecurity concernYes
bugPotential bugYes
breakingBreaking changeYes
标签含义是否阻止合并？
praise突出优秀的实现否
nitpick微小的、可选的建议否
suggestion提出改进方案否
issue需要解决的问题通常是
question请求澄清否
thought供参考的想法否
chore常规任务（格式调整、依赖更新）否
note信息性说明否
todo需要跟进的工作可能
security安全隐患是
bug潜在缺陷是
breaking破坏性变更是

Decorations

修饰符

Optional modifiers in square brackets:
DecorationMeaning
[blocking]Must be addressed before merge
[non-blocking]Optional, can be deferred
[if-minor]Only if it's a quick fix
可选的方括号修饰符：
修饰符含义
[blocking]合并前必须解决
[non-blocking]可选，可延后处理
[if-minor]仅当修改成本极低时建议调整

Examples

示例

typescript
// ✅ Good: Clear, specific, actionable

praise: Excellent use of TypeScript generics here!

This makes the function much more reusable while maintaining type safety.

---

nitpick [non-blocking]: Consider using const instead of let

This variable is never reassigned, so `const` would be more appropriate:
```typescript
const MAX_RETRIES = 3;
issue: Missing error handling for API call
If the API returns a 500 error, this will crash the application. Add a try/catch block:
typescript
try {
  const data = await fetchUser(userId);
  // ...
} catch (error) {
  logger.error('Failed to fetch user', { userId, error });
  throw new UserNotFoundError(userId);
}
question: Why use a Map instead of an object here?
Is there a specific reason for this data structure choice? If it's for performance, could you add a comment explaining?
security [blocking]: API endpoint is not authenticated
The 
/api/admin/users
endpoint is missing authentication middleware. This allows unauthenticated access to sensitive user data.
Add the auth middleware:
typescript
router.get('/api/admin/users', requireAdmin, getUsers);
suggestion [if-minor]: Extract magic number to named constant
Consider extracting this value:
typescript
const CACHE_TTL_SECONDS = 3600;
cache.set(key, value, CACHE_TTL_SECONDS);

---
typescript
// ✅ Good: Clear, specific, actionable

praise: Excellent use of TypeScript generics here!

This makes the function much more reusable while maintaining type safety.

---

nitpick [non-blocking]: Consider using const instead of let

This variable is never reassigned, so `const` would be more appropriate:
```typescript
const MAX_RETRIES = 3;
issue: Missing error handling for API call
If the API returns a 500 error, this will crash the application. Add a try/catch block:
typescript
try {
  const data = await fetchUser(userId);
  // ...
} catch (error) {
  logger.error('Failed to fetch user', { userId, error });
  throw new UserNotFoundError(userId);
}
question: Why use a Map instead of an object here?
Is there a specific reason for this data structure choice? If it's for performance, could you add a comment explaining?
security [blocking]: API endpoint is not authenticated
The 
/api/admin/users
endpoint is missing authentication middleware. This allows unauthenticated access to sensitive user data.
Add the auth middleware:
typescript
router.get('/api/admin/users', requireAdmin, getUsers);
suggestion [if-minor]: Extract magic number to named constant
Consider extracting this value:
typescript
const CACHE_TTL_SECONDS = 3600;
cache.set(key, value, CACHE_TTL_SECONDS);

---

Review Process

评审流程

1. Before Reviewing

1. 评审前准备

Check Context:
  • Read the PR/MR description
  • Understand the purpose and scope
  • Review linked tickets or issues
  • Check CI/CD pipeline status
Verify Automated Checks:
  • Tests are passing
  • Linting has no errors
  • Type checking passes
  • Code coverage meets targets
  • No merge conflicts
Set Aside Time:
  • Small PR (< 200 lines): 15-30 minutes
  • Medium PR (200-500 lines): 30-60 minutes
  • Large PR (> 500 lines): 1-2 hours (or ask to split)
确认上下文：
  • 阅读PR/MR描述
  • 理解变更的目的与范围
  • 查看关联的工单或问题
  • 检查CI/CD流水线状态
验证自动化检查结果：
  • 测试全部通过
  • 代码检查无错误
  • 类型检查通过
  • 代码覆盖率达标
  • 无合并冲突
预留充足时间：
  • 小型PR（<200行）：15-30分钟
  • 中型PR（200-500行）：30-60分钟
  • 大型PR（>500行）：1-2小时（或要求拆分）

2. During Review

2. 评审进行中

Follow a Pattern:
  1. High-Level Review (5-10 minutes)
    • Read PR description and understand intent
    • Skim all changed files to get overview
    • Verify approach makes sense architecturally
    • Check that changes align with stated purpose
  2. Detailed Review (20-45 minutes)
    • Line-by-line code review
    • Check logic, edge cases, error handling
    • Verify tests cover new code
    • Look for security vulnerabilities
    • Ensure code follows team conventions
  3. Testing Considerations (5-10 minutes)
    • Are tests comprehensive?
    • Do tests test the right things?
    • Are edge cases covered?
    • Is test data realistic?
  4. Documentation Check (5 minutes)
    • Are complex sections commented?
    • Is public API documented?
    • Are breaking changes noted?
    • Is README updated if needed?
遵循固定流程：
  1. 高层级评审（5-10分钟）
    • 阅读PR描述并理解意图
    • 快速浏览所有变更文件以掌握整体情况
    • 验证实现方案在架构层面是否合理
    • 确认变更与所述目标一致
  2. 细节评审（20-45分钟）
    • 逐行检查代码
    • 验证逻辑、边界情况与错误处理
    • 确认测试覆盖了新增代码
    • 排查安全漏洞
    • 确保代码符合团队约定
  3. 测试考量（5-10分钟）
    • 测试是否全面？
    • 测试是否覆盖了核心场景？
    • 边界情况是否被覆盖？
    • 测试数据是否贴合实际？
  4. 文档检查（5分钟）
    • 复杂逻辑是否有注释说明？
    • 公共API是否有文档？
    • 破坏性变更是否有标注？
    • 必要时是否更新了README？

3. After Reviewing

3. 评审完成后

Provide Clear Decision:
  • ✅ Approve: Code is ready to merge
  • 💬 Comment: Feedback provided, no action required
  • 🔄 Request Changes: Issues must be addressed before merge
Respond to Author:
  • Answer questions promptly
  • Re-review after changes made
  • Approve when issues resolved
  • Thank author for addressing feedback
给出明确结论：
  • ✅ 批准：代码可合并
  • 💬 评论：提供反馈，无需修改
  • 🔄 要求修改：问题必须解决后才能合并
回复开发者：
  • 及时解答疑问
  • 变更完成后重新评审
  • 问题解决后批准合并
  • 感谢开发者对反馈的响应

Review Checklists

评审检查清单

General Code Quality

通用代码质量

  • Readability: Code is easy to understand
  • Naming: Variables and functions have clear, descriptive names
  • Comments: Complex logic is explained
  • Formatting: Code follows team style guide
  • DRY: No unnecessary duplication
  • SOLID Principles: Code follows SOLID where applicable
  • Function Size: Functions are focused and < 50 lines
  • Cyclomatic Complexity: Functions have complexity < 10
  • 可读性：代码易于理解
  • 命名：变量与函数名称清晰、具有描述性
  • 注释：复杂逻辑有解释说明
  • 格式：代码遵循团队风格指南
  • DRY原则：无不必要的重复代码
  • SOLID原则：代码尽可能遵循SOLID原则
  • 函数规模：函数职责单一且行数<50
  • 圈复杂度：函数圈复杂度<10

Security

安全检查

  • Authentication: Protected endpoints require auth
  • Authorization: Users can only access their own data
  • Input Sanitization: SQL injection, XSS prevented
  • Secrets Management: No hardcoded credentials or API keys
  • Encryption: Sensitive data encrypted at rest and in transit
  • Rate Limiting: Endpoints protected from abuse
  • 身份验证：受保护的接口需要身份验证
  • 权限控制：用户仅能访问自身数据
  • 输入校验：防止SQL注入、XSS攻击
  • 密钥管理：无硬编码凭证或API密钥
  • 加密处理：敏感数据在存储与传输时均加密
  • 限流保护：接口有防滥用的限流措施

Quick Start Guide

快速入门指南

For Reviewers:
  1. Read PR description and understand intent
  2. Check that automated checks pass
  3. Do high-level review (architecture, approach)
  4. Do detailed review (logic, edge cases, tests)
  5. Use conventional comments for clear communication
  6. Provide decision: Approve, Comment, or Request Changes
For Authors:
  1. Write clear PR description
  2. Perform self-review before requesting review
  3. Ensure all automated checks pass
  4. Keep PR focused and reasonably sized (< 400 lines)
  5. Respond to feedback promptly and respectfully
  6. Make requested changes or explain reasoning
Skill Version: 2.0.0 Last Updated: 2026-01-08 Maintained by: AI Agent Hub Team
针对评审者：
  1. 阅读PR描述并理解变更意图
  2. 确认自动化检查全部通过
  3. 进行高层级评审（架构、实现方案）
  4. 进行细节评审（逻辑、边界情况、测试）
  5. 使用标准化评论规范确保沟通清晰
  6. 给出明确结论：批准、评论或要求修改
针对开发者：
  1. 撰写清晰的PR描述
  2. 请求评审前先进行自我评审
  3. 确保所有自动化检查通过
  4. 保持PR聚焦且规模合理（<400行）
  5. 及时且尊重地响应反馈
  6. 根据要求修改代码或解释理由
Skill版本: 2.0.0 最后更新: 2026-01-08 维护团队: AI Agent Hub Team

Related Skills

相关Skill

  • test-standards-enforcer
    - Enforce testing best practices during code review
  • security-scanning
    - Automated security checks to complement manual review
  • unit-testing
    - Unit test patterns to verify during review
  • a11y-testing
    - Accessibility testing requirements for UI code reviews
  • test-standards-enforcer
    - 代码评审期间强制执行测试最佳实践
  • security-scanning
    - 自动化安全检查，补充人工评审
  • unit-testing
    - 评审期间需验证的单元测试模式
  • a11y-testing
    - UI代码评审的可访问性测试要求

Capability Details

能力详情

review-process

review-process

Keywords: code review, pr review, review process, feedback Solves:
  • How to review PRs
  • Conventional comments format
  • Review best practices
关键词: code review, pr review, review process, feedback 解决场景:
  • 如何评审PR
  • 标准化评论格式
  • 评审最佳实践

quality-checks

quality-checks

Keywords: readability, solid, dry, complexity, naming Solves:
  • Check code quality
  • SOLID principles review
  • Cyclomatic complexity
关键词: readability, solid, dry, complexity, naming 解决场景:
  • 代码质量检查
  • SOLID原则评审
  • 圈复杂度校验

security-review

security-review

Keywords: security, authentication, authorization, injection, xss Solves:
  • Security review checklist
  • Find vulnerabilities
  • Auth validation
关键词: security, authentication, authorization, injection, xss 解决场景:
  • 安全评审检查清单
  • 漏洞排查
  • 权限验证

language-specific

language-specific

Keywords: typescript, python, type hints, async await, pep8 Solves:
  • TypeScript review
  • Python review
  • Language-specific patterns
关键词: typescript, python, type hints, async await, pep8 解决场景:
  • TypeScript代码评审
  • Python代码评审
  • 特定语言模式检查

pr-template

pr-template

Keywords: pr template, pull request, description Solves:
  • PR description format
  • Review checklist
关键词: pr template, pull request, description 解决场景:
  • PR描述格式
  • 评审检查清单

Available Scripts

可用脚本

  • scripts/review-pr.md
     - Dynamic PR review with auto-fetched GitHub data
    • Auto-fetches: PR title, author, state, changed files, diff stats, comments count
    • Usage: 
      /review-pr [PR-number]
    • Requires: GitHub CLI (
      gh
      )
    • Uses 
      $ARGUMENTS
      and 
      !command
      for live PR data
  • assets/review-feedback-template.md
     - Static review feedback template
  • assets/pr-template.md
     - PR description template
  • scripts/review-pr.md
     - 自动拉取GitHub数据的动态PR评审脚本
    • 自动拉取内容：PR标题、作者、状态、变更文件、差异统计、评论数
    • 使用方式: 
      /review-pr [PR-number]
    • 依赖: GitHub CLI (
      gh
      )
    • 利用
      $ARGUMENTS
      和
      !command
      获取实时PR数据
  • assets/review-feedback-template.md
     - 静态评审反馈模板
  • assets/pr-template.md
     - PR描述模板