code-review-practices

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review Practices

代码审查实践

This skill provides guidance for conducting effective, constructive code reviews that improve code quality and team collaboration.
本技能为开展高效、具建设性的代码审查提供指导,以提升代码质量并促进团队协作。

Review Mindset

审查心态

Goals of code review:
  • Catch bugs and issues early
  • Share knowledge across the team
  • Maintain code quality standards
  • Improve code maintainability
  • Foster learning and growth
Not goals:
  • Assert dominance or superiority
  • Enforce personal preferences
  • Block progress unnecessarily
  • Nitpick without value
代码审查的目标:
  • 尽早发现漏洞与问题
  • 在团队内共享知识
  • 维持代码质量标准
  • 提升代码可维护性
  • 促进学习与成长
非目标:
  • 彰显主导地位或优越感
  • 强制执行个人偏好
  • 不必要地阻碍进度
  • 做无价值的吹毛求疵

What to Review

审查内容

1. Correctness

1. 正确性

Does the code work as intended?
  • Logic is sound
  • Edge cases are handled
  • Error conditions are managed
  • Requirements are met
代码是否按预期工作?
  • 逻辑严谨
  • 处理了边缘情况
  • 管理了错误场景
  • 满足需求

2. Design and Architecture

2. 设计与架构

Does it fit the system?
  • Follows existing patterns
  • Appropriate abstractions
  • Reasonable complexity
  • Scalable approach
是否适配现有系统?
  • 遵循现有模式
  • 采用合适的抽象
  • 复杂度合理
  • 具备可扩展的实现方式

3. Readability

3. 可读性

Can others understand it?
  • Clear naming
  • Logical organization
  • Appropriate comments
  • Consistent style
他人能否轻松理解?
  • 命名清晰
  • 组织逻辑合理
  • 注释恰当
  • 风格一致

4. Testing

4. 测试

Is it adequately tested?
  • Tests exist for new code
  • Edge cases covered
  • Tests are maintainable
  • Integration points tested
测试是否充分?
  • 新增代码配有测试
  • 覆盖了边缘情况
  • 测试易于维护
  • 测试了集成点

5. Security

5. 安全性

Are there security concerns?
  • Input validation
  • Authentication/authorization
  • Sensitive data handling
  • Known vulnerabilities
是否存在安全隐患?
  • 输入验证
  • 认证/授权机制
  • 敏感数据处理
  • 已知漏洞排查

6. Performance

6. 性能

Will it perform well?
  • No obvious bottlenecks
  • Efficient algorithms
  • Resource usage reasonable
  • Scalability considered
性能是否达标?
  • 无明显性能瓶颈
  • 算法高效
  • 资源使用合理
  • 考虑了可扩展性

Review Process

审查流程

1. Understand Context

1. 理解上下文

Before reviewing code:
  • Read PR description and linked issues
  • Understand the problem being solved
  • Check CI/CD results
  • Note any special considerations
审查代码前:
  • 阅读PR描述及关联议题
  • 理解要解决的问题
  • 查看CI/CD结果
  • 留意特殊注意事项

2. Review at Appropriate Level

2. 分层次审查

High-level first:
  • Overall approach
  • Architecture decisions
  • Major design choices
Then details:
  • Implementation specifics
  • Edge cases
  • Code style
先做高层级审查:
  • 整体实现思路
  • 架构决策
  • 主要设计选择
再做细节审查:
  • 具体实现细节
  • 边缘情况
  • 代码风格

3. Provide Actionable Feedback

3. 提供可落地的反馈

Good feedback:
  • Specific and clear
  • Explains the "why"
  • Suggests solutions
  • Prioritizes issues
Example:
Consider using a Set instead of an array here for O(1) lookups. 
With the current implementation, the nested loop creates O(n²) 
complexity which could be problematic with large datasets.
优质反馈:
  • 具体清晰
  • 解释“原因”
  • 给出解决方案建议
  • 区分问题优先级
示例:
Consider using a Set instead of an array here for O(1) lookups. 
With the current implementation, the nested loop creates O(n²) 
complexity which could be problematic with large datasets.

4. Use Review Comments Effectively

4. 合理使用审查评论

Types of comments:
Blocking (must fix):
  • "This will cause a race condition when..."
  • "Security issue: User input is not sanitized..."
  • "This breaks the API contract by..."
Non-blocking (suggestions):
  • "Consider: This could be simplified by..."
  • "Nit: Variable name could be more descriptive"
  • "Question: Why did you choose X over Y?"
Positive:
  • "Nice solution! This is much cleaner than..."
  • "Great test coverage on the edge cases"
  • "I learned something new from this approach"
评论类型:
阻塞型(必须修复):
  • “这会在……场景下引发竞态条件”
  • “安全问题:用户输入未经过滤……”
  • “这违反了API契约,具体表现为……”
非阻塞型(建议):
  • “建议:可通过……方式简化此代码”
  • “小建议:变量名可更具描述性”
  • “疑问:为什么选择方案X而非Y?”
肯定型:
  • “解决方案很棒!这比……简洁得多”
  • “边缘情况的测试覆盖很全面”
  • “从这个实现中我学到了新东西”

5. Distinguish Preferences from Problems

5. 区分客观问题与主观偏好

Problems (objective):
  • Bugs and logic errors
  • Security vulnerabilities
  • Performance issues
  • Breaking changes
Preferences (subjective):
  • Code style choices
  • Naming preferences
  • Organization approaches
  • Minor optimizations
Rule: Block on problems, discuss preferences.
客观问题:
  • 漏洞与逻辑错误
  • 安全漏洞
  • 性能问题
  • 破坏性变更
主观偏好:
  • 代码风格选择
  • 命名偏好
  • 代码组织方式
  • 次要优化点
原则: 因客观问题阻塞合并,针对主观偏好展开讨论。

Communication Guidelines

沟通准则

Be Constructive

保持建设性

Instead of:
  • "This is wrong"
  • "Why would you do it this way?"
  • "This code is terrible"
Say:
  • "This could cause X issue because..."
  • "Have you considered Y approach? It might..."
  • "This could be improved by..."
避免:
  • “这是错的”
  • “你为什么要这么做?”
  • “这段代码糟透了”
建议表述:
  • “这可能会导致X问题,原因是……”
  • “你是否考虑过Y方案?它或许可以……”
  • “这部分可通过……方式优化”

Explain Your Reasoning

解释你的理由

Weak comment:
This should be refactored.
Strong comment:
Consider extracting this logic into a separate method. 
It's used in three places and would be easier to test 
and maintain as a standalone function.
薄弱的评论:
This should be refactored.
有力的评论:
Consider extracting this logic into a separate method. 
It's used in three places and would be easier to test 
and maintain as a standalone function.

Ask Questions

主动提问

Use questions to understand and guide:
  • "What happens if X is null here?"
  • "Could this be simplified using Y?"
  • "Have you considered the case where...?"
通过提问来理解并引导:
  • “如果此处X为null会发生什么?”
  • “是否可以用Y来简化这部分?”
  • “你是否考虑过……的场景?”

Acknowledge Good Work

认可优质工作

Don't only point out problems:
  • "Clever use of X to solve Y"
  • "Great test coverage"
  • "This is much cleaner than the old approach"
  • "Nice documentation"
不要只指出问题:
  • “巧妙地用X解决了Y问题”
  • “测试覆盖很全面”
  • “这比旧实现简洁多了”
  • “文档写得很棒”

Be Humble

保持谦逊

You might be wrong:
  • "I might be missing something, but..."
  • "Could you explain why...?"
  • "I'm not familiar with X, but..."
你可能也会出错:
  • “我可能忽略了某些点,但……”
  • “能否解释一下为什么……?”
  • “我对X不太熟悉,但……”

Common Review Patterns

常见审查模式

Code Smells to Watch For

需要关注的代码异味

Long methods/functions:
  • Hard to understand
  • Difficult to test
  • Multiple responsibilities
Duplicated code:
  • Maintenance burden
  • Inconsistent behavior
  • Missed bug fixes
Complex conditionals:
  • Hard to reason about
  • Error-prone
  • Difficult to test
Large classes:
  • Too many responsibilities
  • Hard to maintain
  • Tight coupling
Magic numbers/strings:
  • Unclear meaning
  • Hard to change
  • Error-prone
过长的方法/函数:
  • 难以理解
  • 难以测试
  • 承担多重职责
重复代码:
  • 维护负担重
  • 行为不一致
  • 遗漏漏洞修复
复杂条件判断:
  • 难以推理
  • 容易出错
  • 难以测试
过大的类:
  • 职责过多
  • 难以维护
  • 耦合紧密
魔法数字/字符串:
  • 含义模糊
  • 难以修改
  • 容易出错

Security Red Flags

安全警示信号

  • Hardcoded credentials or secrets
  • SQL injection vulnerabilities
  • XSS vulnerabilities
  • Missing input validation
  • Insecure data storage
  • Weak authentication
  • Missing authorization checks
  • 硬编码的凭证或密钥
  • SQL注入漏洞
  • XSS漏洞
  • 缺少输入验证
  • 不安全的数据存储
  • 弱认证机制
  • 缺少授权校验

Performance Concerns

性能隐患

  • N+1 query problems
  • Inefficient algorithms (O(n²) when O(n) possible)
  • Memory leaks
  • Unnecessary database calls
  • Large data structures in memory
  • Blocking operations in async code
  • N+1查询问题
  • 低效算法(可用O(n)却用了O(n²))
  • 内存泄漏
  • 不必要的数据库调用
  • 内存中的大型数据结构
  • 异步代码中的阻塞操作

Review Standards

审查标准

Establish Team Norms

建立团队规范

Define standards for:
  • Code style and formatting
  • Naming conventions
  • Comment requirements
  • Test coverage expectations
  • Documentation needs
Document standards:
  • Style guides
  • Architecture decision records
  • Review checklists
  • Example code
明确以下标准:
  • 代码风格与格式
  • 命名约定
  • 注释要求
  • 测试覆盖率预期
  • 文档需求
将标准文档化:
  • 风格指南
  • 架构决策记录
  • 审查清单
  • 代码示例

Review Checklist

审查清单

Use a consistent checklist:
  • Code is correct and handles edge cases
  • Tests exist and pass
  • No security vulnerabilities
  • Performance is acceptable
  • Code is readable and maintainable
  • Documentation is updated
  • No breaking changes (or properly communicated)
  • Follows team conventions
使用统一的审查清单:
  • 代码正确且处理了边缘情况
  • 测试已存在且通过
  • 无安全漏洞
  • 性能达标
  • 代码可读且易于维护
  • 文档已更新
  • 无破坏性变更(或已充分沟通)
  • 遵循团队约定

Handling Disagreements

处理分歧

When You Disagree

当你持有不同意见时

If it's a preference:
  • State your opinion
  • Explain your reasoning
  • Accept the author's choice
  • Move on
If it's a problem:
  • Clearly explain the issue
  • Provide evidence or examples
  • Suggest alternatives
  • Escalate if needed
如果是主观偏好:
  • 表明你的观点
  • 解释理由
  • 接受作者的选择
  • 继续推进
如果是客观问题:
  • 清晰说明问题
  • 提供证据或示例
  • 给出替代方案
  • 必要时升级处理

When Author Disagrees

当作者持有不同意见时

Listen and understand:
  • They may have context you lack
  • There might be constraints you don't know
  • Your suggestion might not fit
Discuss, don't dictate:
  • Explain your concerns
  • Ask questions
  • Find common ground
  • Compromise when appropriate
倾听并理解:
  • 他们可能掌握你不知道的上下文
  • 可能存在你不了解的约束条件
  • 你的建议可能不适用
展开讨论,而非发号施令:
  • 说明你的顾虑
  • 提出问题
  • 寻求共识
  • 适当妥协

Review Timing

审查时机

Response Time

响应时间

Aim for:
  • Small PRs: Within 4 hours
  • Medium PRs: Within 1 day
  • Large PRs: Within 2 days
If you can't review promptly:
  • Let the author know
  • Suggest another reviewer
  • Set expectations
目标:
  • 小型PR:4小时内
  • 中型PR:1天内
  • 大型PR:2天内
若无法及时审查:
  • 告知作者
  • 推荐其他审查者
  • 明确预期时间

Review Size

审查规模

Optimal PR size:
  • 200-400 lines of code
  • Focused on single concern
  • Reviewable in 30-60 minutes
For large PRs:
  • Request breakdown if possible
  • Review in multiple passes
  • Focus on high-level first
  • Consider pair review
最优PR规模:
  • 200-400行代码
  • 聚焦单一关注点
  • 30-60分钟可完成审查
针对大型PR:
  • 如有可能,请求拆分
  • 分多次审查
  • 先聚焦高层级内容
  • 考虑结对审查

Mentoring Through Reviews

通过审查进行指导

Teaching Opportunities

教学机会

Use reviews to share knowledge:
  • Explain patterns and practices
  • Share relevant resources
  • Demonstrate better approaches
  • Encourage questions
利用审查分享知识:
  • 讲解模式与实践
  • 分享相关资源
  • 演示更优实现方式
  • 鼓励提问

Growing Reviewers

培养审查者

Help others become better reviewers:
  • Invite junior developers to review
  • Explain your review comments
  • Discuss review decisions
  • Share review best practices
帮助他人提升审查能力:
  • 邀请初级开发者参与审查
  • 解释你的审查评论
  • 讨论审查决策
  • 分享审查最佳实践

Anti-Patterns to Avoid

需避免的反模式

Nitpicking without value - Focus on meaningful improvements ❌ Blocking on style preferences - Use automated tools instead ❌ Rewriting in your style - Respect different approaches ❌ Reviewing too quickly - Take time to understand ❌ Being overly critical - Balance criticism with praise ❌ Ignoring context - Consider constraints and tradeoffs ❌ Making it personal - Focus on code, not the person
无价值的吹毛求疵 - 聚焦有意义的改进 ❌ 因风格偏好阻塞合并 - 改用自动化工具处理 ❌ 按自己的风格重写代码 - 尊重不同实现方式 ❌ 过快完成审查 - 花时间理解上下文 ❌ 过度批评 - 平衡批评与表扬 ❌ 忽略上下文 - 考虑约束条件与权衡 ❌ 针对个人 - 聚焦代码而非开发者

Review Outcomes

审查结果

Approve

批准

Code meets standards and is ready to merge:
  • All critical issues addressed
  • Tests pass
  • Documentation updated
  • No blocking concerns
代码符合标准,可合并:
  • 所有关键问题已解决
  • 测试通过
  • 文档已更新
  • 无阻塞性问题

Approve with Comments

附评论批准

Minor issues that don't block merge:
  • Style suggestions
  • Future improvements
  • Questions for discussion
  • Nice-to-have changes
存在不影响合并的次要问题:
  • 风格建议
  • 未来优化方向
  • 待讨论的问题
  • 可选的改进点

Request Changes

请求修改

Issues that must be addressed:
  • Bugs or logic errors
  • Security vulnerabilities
  • Missing tests
  • Breaking changes
  • Architectural concerns
存在必须解决的问题:
  • 漏洞或逻辑错误
  • 安全漏洞
  • 缺少测试
  • 破坏性变更
  • 架构问题

Reject (Rare)

拒绝(罕见)

Fundamental problems requiring redesign:
  • Wrong approach entirely
  • Doesn't solve the problem
  • Creates major technical debt
  • Violates core principles
存在需要重新设计的根本性问题:
  • 实现思路完全错误
  • 未解决目标问题
  • 引入大量技术债务
  • 违反核心原则

Quick Reference

快速参考

Good review comment structure:
  1. Identify the issue
  2. Explain why it's a problem
  3. Suggest a solution
  4. Indicate severity (blocking vs suggestion)
Example:
[Blocking] This query will cause N+1 problem when loading 
related records. Consider using eager loading with includes() 
to fetch all data in a single query. This will significantly 
improve performance with large datasets.
Remember:
  • Be kind and constructive
  • Focus on the code, not the person
  • Explain your reasoning
  • Acknowledge good work
  • Know when to compromise
  • Foster learning and growth
优质审查评论结构:
  1. 指出问题
  2. 解释问题的影响
  3. 给出解决方案建议
  4. 说明严重程度(阻塞型/建议型)
示例:
[Blocking] This query will cause N+1 problem when loading 
related records. Consider using eager loading with includes() 
to fetch all data in a single query. This will significantly 
improve performance with large datasets.
谨记:
  • 友善且具建设性
  • 聚焦代码而非个人
  • 解释你的理由
  • 认可优质工作
  • 懂得适时妥协
  • 促进学习与成长