mcpField
E
engineering-code-review

engineering-code-review

github.com/agentara测试与质量619122 次安装4 周前更新
代码仓库

关于 engineering-code-review

这项代码审查技能依据代码健康、正确性、可维护性和风险评估变更，并提供结构化流程、检查清单、结论指引及寻求专业审查的条件。

主要功能

  1. 01明确审查范围并收集背景
  2. 02评估设计与代码
  3. 03检查风险和测试
  4. 04提供审查结论指引
  5. 05明确专业审查升级条件

适用场景

  1. 01审查 GitHub 拉取请求
  2. 02评估本地代码差异
  3. 03检查安全敏感领域的变更

安装

复制下面的命令，在终端中运行。

$npx skills add https://github.com/agentara/skills --skill engineering-code-review

安装后在 Claude Code 中输入/engineering-code-review确认已加载。

详细介绍

工程代码审查

通过代码审查保护并改善代码的长期健康状况，同时让有价值的更改顺利合入。标准不是追求完美，而是确保更改能整体改善系统，且不会引入不可接受的风险。

改编自 Google Engineering Practices Documentation，尤其是代码审查概述、审查者指南、审查标准、导航、速度、小型更改和紧急情况指南。来源：https://google.github.io/eng-practices/、https://google.github.io/eng-practices/review/reviewer/standard.html、https://google.github.io/eng-practices/review/reviewer/navigate.html、https://google.github.io/eng-practices/review/reviewer/speed.html、https://google.github.io/eng-practices/review/developer/small-cls.html 和 https://google.github.io/eng-practices/review/emergencies.html。许可证：CC-BY 3.0。

审查标准

  • 如果更改确实改善了所涉及系统的代码健康状况，即使不完美，也应倾向于批准。
  • 除非确属紧急情况且明确安排了后续清理，否则不要批准会使系统变差的更改。
  • 相较于个人偏好，应优先考虑技术事实、数据、现有项目标准和经得起时间考验的设计原则。
  • 将风格指南和自动格式化工具视为权威依据。个人风格偏好只是细枝末节。
  • 如果有多种合理的设计方案，除非作者偏好的方案会损害代码健康状况，否则应予以接受。
  • 如果更改涉及安全、隐私、并发、无障碍、国际化、基础设施、数据迁移或你无法验证的领域逻辑等专业领域，请邀请另一位具备相应资质的审查者。

审查工作流

  1. 明确范围。

    • 确定差异、要求的审查范围、目标分支，以及你进行的是完整审查还是部分审查。
    • 如果是部分审查，请明确说明你审查了哪些文件、行为或问题。

  2. 收集审查材料。

    • 对于 GitHub PR，在发表评论前检查 PR 描述、关联的问题单／设计文档、变更文件、现有审查意见、CI 状态和相关提交历史。
    • 对于本地审查，检查 git status --short、已知的目标分支或基准 SHA，以及本地差异。
    • 如果看不到差异或 PR 上下文，请索取相关信息，而不是凭空编造审查意见。

  3. 先从整体着手。

    • 阅读描述、问题单、测试和周边上下文。
    • 判断这项变更现在是否应该存在于此代码库中。
    • 如果方向有误，尽早指出并建议更好的方案。
    • 如果变更过大，无法彻底审查，请先要求拆分，或制定经审查者认可的大型变更计划，再尝试进行浅层完整审查。

  4. 先审查主要设计，再看细节。

    • 找出定义这项变更的核心文件或 API。
    • 在提出命名或格式方面的意见之前，先审查架构、职责边界、数据流、兼容性和回滚风险。
    • 如果主要设计问题会使其余审查失去意义，请立即提出。

  5. 审查指定范围内每一行人工编写的内容。

    • 适当情况下，可抽样检查生成文件、大型数据文件或机械生成的输出。
    • 如果代码太难理解，请在批准前要求简化或改进结构，使其更清晰。

  6. 根据下方的审查清单检查变更。

  7. 检查验证依据。

    • 如果用户没有要求你运行命令，优先查看现有 CI 和针对性测试。
    • 只有在环境和请求适合的情况下才运行本地测试；否则说明缺少或尚未验证哪些测试。
    • 除非你看到了命令输出或 CI 结果，否则不要声称测试已通过。

  8. 给出结论。

    • APPROVE：代码健康状况有所改善，剩余意见不会阻碍通过。
    • APPROVE_WITH_COMMENTS：剩余意见较小、可选，或作者可以在无需再进行一轮审查的情况下自行处理。
    • REQUEST_CHANGES：仍存在正确性、设计、可维护性、测试、文档或风险方面的问题。
    • NEEDS_SPECIALIST：必须由你无法负责评估该领域的另一位审查者进行审查。

审查清单

  • 设计：这项变更适合放在这里吗？抽象是否适合当前系统？
  • 功能：它是否实现了作者的意图？这种行为对用户和未来的开发者来说是否合适？
  • 边界情况：是否考虑了故障、空状态、重试、幂等性、顺序、时间、权限和并发？
  • 复杂性：能否快速理解？它解决的是当前已知问题，而非对未来的猜测吗？
  • 测试：根据风险，是否适合采用单元、集成或端到端测试？如果代码有问题，测试能否失败？
  • 测试质量：断言是否有意义、聚焦、确定且易于维护？
  • 命名：名称是否清晰表达用途，避免含糊或冗余？
  • 注释：注释是否解释原因、约束或不明显的算法，而非复述代码？
  • 文档：当行为或用法发生变化时，是否更新了 README、API 文档、迁移说明、运行手册或生成的文档？
  • 风格与一致性：变更是否遵循项目约定，且没有在功能性工作中混入无关的格式调整？
  • 上下文：这项变更是否改善了周边系统，还是又增加了一小块长期复杂性？
  • 运维：部署、迁移、可观测性、回滚、功能标志和故障处理是否足以应对这项变更？
  • 安全：是否妥善处理了授权、身份验证、密钥、注入、依赖项、反序列化和权限边界风险？
  • 隐私：这项变更是否避免了不必要地收集、记录、暴露、保留敏感数据，或将其跨边界传输？
  • 无障碍：对于用户界面，是否保留了键盘访问、语义、焦点、对比度、标签和辅助技术行为？
  • 国际化：在相关情况下，是否妥善处理了区域设置、时区、编码、文本扩展、复数形式、排序和翻译后的字符串？

如果无法有把握地评估安全、隐私、无障碍、国际化、并发、基础设施、数据迁移或特定领域的正确性，请升级给专家处理。

评论严重级别

  • BLOCKER：批准前必须修复。
  • IMPORTANT：批准前应修复，除非有书面理由。
  • QUESTION：作出最终结论前需要答复。
  • NIT：润色建议，不得因此阻止批准。
  • OPTIONAL：作者可以不采纳的建议。
  • FYI：用于学习或供未来参考，并非要求变更。

输出格式

先列出发现的问题，按严重程度从高到低排序。有可用的文件和行号时，请注明。

## Findings
- [BLOCKER] path/file.ext:123 - Problem. Why it matters. Required change.
- [IMPORTANT] path/file.ext:45 - Problem. Why it matters. Suggested fix.

## Open Questions
- ...

## Verdict
REQUEST_CHANGES | APPROVE_WITH_COMMENTS | APPROVE | NEEDS_SPECIALIST

## Notes
- Scope reviewed:
- Tests/commands checked:
- Follow-ups:

如果没有发现问题，请明确说明，并仍然提及任何未经验证的范围或残余风险。

常见错误

  • 因个人偏好而非代码健康问题阻止批准。
  • 只审查变更的行，而周边函数或模块可能使这项变更不安全。
  • 仅仅因为作者在审查讨论串中作了解释，就接受复杂代码。
  • 对于变更本身引入的复杂性，放任“以后再整理”。
  • 到后期才要求大规模重写，而没有及早指出设计问题。
  • 明明快速给出概括性答复就能推进有价值的工作，却让作者一直等待。

评论

支持 Markdown