code-review-practices
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseCode 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:
- Identify the issue
- Explain why it's a problem
- Suggest a solution
- 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
优质审查评论结构:
- 指出问题
- 解释问题的影响
- 给出解决方案建议
- 说明严重程度(阻塞型/建议型)
示例:
[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.谨记:
- 友善且具建设性
- 聚焦代码而非个人
- 解释你的理由
- 认可优质工作
- 懂得适时妥协
- 促进学习与成长