代码审查清单

我在上一篇博客文章中介绍了我们如何进行代码审查，并将其与结对编程进行了比较。

我注意到，每个团队和团队成员在进行代码审查时，关注的重点都不尽相同。在这篇文章中，我将列出我通常会关注的内容。但这绝不是一个完整的清单。如果您认为还有其他遗漏，请在评论区补充。

设计

根据你使用的编程语言或平台，检查新代码是否遵循正确的设计。

什么是正确的设计需要团队协商一致。引入新的抽象概念并遵循新的设计方案时，最好事先在开发人员之间进行讨论。代码审查可以确保设计按照讨论的内容实现，从而避免大量的返工。但如果你想实现一些新功能并收集反馈，则可以省略代码审查。务必确保编写足够的文档来清晰地传达你的想法。

以下是一些你应该检查的事项（感谢Bhavin Javia为这份清单的大部分内容做出的贡献）：

• 检查是否遵循指定风格指南中的编码规范，以保持一致性和最佳实践。
• 检查开发人员是否无意中引入了重复代码，例如由于缺乏对现有可重用组件的了解、逻辑/职责错置、通用/可重用代码未被提取出来而直接复制粘贴到其他地方等原因造成的重复代码。
• 找出之前技术债务审查和代码审查中遗漏或延后的重构机会。通常情况下，如果开发人员正在该领域工作，他们掌握很多相关背景信息，因此更容易将一些重构任务融入到用户故事/PR中，而无需专门安排时间进行重构。
• 发现并提醒开发人员注意与其他开发人员代码的潜在依赖关系或冲突，例如，当多个功能正在开发或审查时，如果这些功能涉及代码库的相同区域。
• 如果引入了任何非标准功能或临时解决方案，请务必使用 #TODO 或 #FIXME 标签进行标注，并附上相应的注释，例如引用所使用的代码片段/补丁，说明原因以及理想的解决方案。例如：#FIXME 补丁，用于解决 <framework> 中的 bug # <link>。升级到修复版本后移除。
• 留意新推出的库，看看是否是必需的，以及是否是最新版本。是否有更好的替代方案？

测试

• 以上所有关于代码的检查清单也适用于测试代码。
• 确保所有代码都编写了各个级别的测试——单元测试、集成测试和功能测试。
• 检查所有测试是否都通过。
• 检查测试是否起到文档作用，描述了被测代码的意图。
• 检查测试失败时是否给出正确的错误信息。例如：

expect(subject).to include(:customer) #is better than:
expect(subject.key? :customer).to be\_true

戴上质量保证帽

这一点常常被大多数人忽略，或者他们认为它并不重要。代码的运行效率与代码质量同等重要。即使你拥有测试团队，在代码审查阶段发现并修复 bug 也能降低修复成本。

• 阅读用户故事描述，向业务分析师或产品负责人提问，确认所有验收标准是否都已按要求实施。我认为仅凭这一步就能发现大部分缺陷。
• 思考一下故事之外的其他场景。检查一下与新变化相关的常规用户流程是否仍然按预期运行。

跨职能需求（非功能性需求）

同时关注性能、安全、分析、日志记录、警报等跨职能需求。

• 检查代码是否会导致性能问题，例如 N+1 查询问题或将整个数据库加载到内存中等。设想一下在生产环境中运行这段代码，并预测可能会出现哪些问题。当然，你不可能仅通过查看代码就发现所有问题，但你或许能够根据过去的经验识别出一些常见的错误。例如，执行 Customer.all。
• 安全问题可以及早发现。检查数据是否仅限授权用户访问。了解常见的安全防范措施，并与团队分享这些知识。
• 如果您已经与某些分析系统集成，请检查新功能是否需要与该系统集成。
• 检查日志记录是否足够，以便能够调试应用程序。
• 检查是否已添加故障警报。

持续交付

遵循持续交付原则时，应确保新的变更不会破坏生产数据和功能。

• 请确保已添加数据迁移脚本，并检查其是否按预期执行。如有必要，请在您的机器上运行一次。切勿进行任何会导致数据丢失的更改。此外，请确保即将上线生产环境的功能已自动部署到预发布或测试环境中，该环境应包含生产数据副本（已进行混淆处理），以便检测和修复潜在问题。
• 检查是否为任何您不打算发布的功能添加了功能开关。

其他

• 检查待审核的代码是否已合并到最新的主分支代码中。
• 确保添加相关说明，以便其他团队在集成这些变更后遵循。例如，告知质量保证团队在发生重大变更时应在哪些区域运行回归测试；或者告知下游用户系统 API 协议的任何变更；或者更新 README 文件以帮助其他开发人员。

总的来说，要尽量减少需要审查的更改，并尽快提交 pull request 以便尽早获得反馈。

请在评论区留下您的想法。如果您喜欢这篇文章，请推荐给其他人，让他们也能看到并参与讨论。