代码审查最佳实践

原文：《Code Review Best Practices》 作者：Trisha Gee

作为开发者，我们都知道代码审查理论上是一件好事儿，它可以帮助我们：

• 尽早发现缺陷和安全隐患。
• 提升代码的可靠性。
• 为确保所有任务都已完成提供了安全保障。

但实际上，代码审查往往给参与者带来的是一种不舒服的体验。导致更多的争论、无效审查，更甚至是不再做审查。

这里提供了一个帮助大家建立一条有效的代码审查流程的指南。

为什么做代码审查？

在代码审查的审查过程中，第一个需要回答的问题是：我们代码审查的目的是什么？当你问这个问题的时候，你将很快意识到做代码审查的原因很多，甚至你会发现团队中的每个人的答案都不一样。他们可能认为自己审查是为了：

• 找出缺陷。
• 检查潜在的性能或安全问题。
• 确认功能的完整性。
• 确保设计的合理性。
• 分享实现的新特性和设计变更。
• 检查代码是否符合标准。
• 以及另外的一千个理由。

如果团队中每个人的答案都不一样，他们在代码中关注的焦点也不一样。这种情况可能会产生一些反作用：

• 由于每个审查人都有提出了一大堆不同的问题等待处理而消耗很长的时间完成代码审查。
• 审查者变得缺乏动力，因为每次审查都会根据审查者的不同而引发不同类型的问题。
• 由于每次更新代码都产生新的问题，审查人和代码作者可能因此开始“打乒乓球”。

确保代码的作者和审查者等所有的代码审查参与人拥有唯一的审查目的可以确保大家的焦点和建议更加切合这个目的。

都审查些什么？

只有我们明白为什么做代码审查，才能清晰的知道我们要审查什么。我们知道在代码审查的过程中有很多东西被审查，我们需要减小这个范围，只关注我们真正关心的内容。

例如，我们做代码审查的目的是保证代码的易读性，那么就不应该花过多时间在某个设计是否实现，而应该更多的关注我们是否能理解这个方法以及这个方法声明是否合理。这种特定的选择带来的好处是随着代码的易读性提高，我们发现异常或错误的逻辑更加容易，而且往往简单的代码性能也会更高。

我们应该尽可能的采用自动化流程（sonar等），人工代码复查就可以不关注以下内容：

• 代码格式规范检查。
• 测试覆盖率。
• 是否满足性能规范要求。
• 常规安全问题。

其实，人工代码审查最终关注的问题可能非常简单：

• 可读性。
• 可维护性。
• 扩展性。

这些都是不能自动化的检查，并且是对开发人员影响深远的代码特性。

当然开发人员并不是重要的，代码的最终目的是投入生产。所以业务上最关心的还是：这段代码是否达到了设计目的？是否有一个自动化测试或者测试集来保证？

最后，它是否满足所有的非功能性需求？这些检测中监督性要求（如审计）和用户要求（如文档）也是非常重要的。

哪些人参与审查？

带着明确的目的和清晰的规则去做代码审查，确认那些人参与审查就变得容易多了。我们需要确定一下角色：

谁来做审查？

大多数人认为参与审查的应该是一个或多个资深或经验丰富的开发人员。但是如果是聚焦于代码的易读性时，初级开发者可能才是正确的人选——一个经验缺乏的开发者能看懂的代码其他人应该也能看懂。如果是审查共享的知识，我们希望每个人都能阅读这份代码，为了这个目的，我们应该有一个审查人备选群体，然后随机选中一部分来参与审查工作。

谁来结束审查？

当我们有多个审查人的时候，谁来结束审查工作就变得非常重要了。这个角色可以是某一个人、特定的一组人、一定数量的审查人或该领域的专家，甚至是一个拥有一票否决权的人来终止审查流程。在具有较高信任度的团队中，代码作者则可能是确定回馈是否足够和问题是否完全解决的人。

谁来冲裁争议？

在多个人一起审查代码的时候，如果不同的审查人之间出现意见冲突，谁来解决？由作者决定？由领导或者经验丰富的同事（专家）来仲裁，并确定最佳方案？理解如何处理代码审查期间的意见冲突是非常重要。

有哪些时间节点？

代码审查的时间主要包含两个部分：

什么时候做？

开始传统的代码审查工作是在代码开发完成即将发布生产的时候进行，通常审查完成之前代码不会合并到主分支，比如：PR模型。但这不是唯一的方法，如果代码审查时为了知识共享，那么审查可以在代码合并之后进行（或者直接合并至主分支）。如果代码审查是审查被认为改进代码设计的增量审查时，审查工作将在代码实现的过程中进行。一旦我们明确为什么审查、审查什么和那些人参与的时候，我们就能更容易确定什么时候是审查的最佳时间。

什么时候结束？

不明确什么时候可以结束审查流程是导致代码审查无限延期的主要原因。没什么是比无休止的干一件事儿更让人泄气了，开发者感觉他们一直在做相同的事儿，却一直没有投入生产（这让我想起了刚开始工作的时候，从事互联网开发的时候，真的很沮丧）。原则上决定是什么时候结束评审取决于参与评审的人员和评审时间：

• 对于知识共享审查，只要内容所有人都能看的时候就可以结束了。
• 对于网关审查，当所有观点都解决之后，由一位高级主管（看门人）确定审查结束。
• 其他类型的审查在完成之前也许有一系列的条件需要满足，例如：
  • 所有的评论都通过修改代码来解决了。
  • 所有的评论都引起了代码的修改或者一个issue（如：新特性或设计变更；新功能的附加信息；技术债务等）。
  • 标记为阻断性问题的评论全部通过某种方法解决，那些经验性、总结性的评论则不需要被标记解决。

在什么地方做？

代码审查不一定非要在某种工具中进行，结对编程就是代码审查的一种形式。代码审查可以将同事拉倒一起，和他一起阅读你的代码；代码审查也可以切出一个分支，然后通过文档、邮箱或聊天工具发表评论完成；代码审查还可以通过Github的PR或者其他的审查工具完成。

总结

在做代码审查的时候有很多事情需要考虑，如果想要面面俱到，最终几乎不可能完成代码审查过程。只有适合自己的才是最好的，可以从以下几个方面考虑什么才是最适合自己的：

• 为什么我们要做代码审查？

  如果有明确的目标，审查者可以更容易的完成工作；代码作者也能更愉快的响应审查者的问题。

• 我们主要审查什么？

  当我们有一个目标后，我们可以创建一些列更加精确的标准来进行代码审查。

• 需要那些人参与审查工作？

  谁来做审查？谁来负责处理冲突意见？当代码通过审查时谁来结束流程？

• 什么时候开始？什么时候结束？

  在代码编写过程中或者完成后，审查工作可以迭代进行。如果没有一个明确审查标准说明什么样的代码可以通过时，审查工作有可能有无止境无法完成。

• 我们在什么地方做代码审查？

  代码审查不需要特定的工具，甚至可以在工位上和同事一起完成。

当这几个问题思考清楚后，就可以建立一套良好的适合当前团队的代码审查流程。重点：代码复查的目标是投入生产，而不是秀技术！