提交后审查

我最近在Amazon Builder‘s Library上阅读了Clare Liguori撰写的一篇很棒的文章，其中非常详细地介绍了AWS的CI/CD架构。这是一篇非常精彩的文章，我建议所有对CI/CD基础设施感兴趣的人阅读这篇文章。本文涵盖了从单元测试到集成测试(在生产中)，到“WAVE”中的阶段性部署，再到自动回滚的方方面面。我保证，即使您不像AWS那样构建世界上最大的分布式系统，您也会从这篇文章中学到一些有价值的东西。

这篇文章唯一让我惊讶的是，它说在合并到主分支之前，代码审查是强制性的。

所有要投入生产的更改都是从代码审查开始的，在合并到主线分支(我们的版本为“Main”或“Trunk”)之前，必须得到团队成员的批准，主线分支会自动启动管道。管道强制要求主线分支上的所有提交必须由该管道的服务团队成员进行代码审查和批准。管道将阻止部署任何未经审查的提交。

对于全自动管道，代码审查是代码更改在部署到生产之前从工程师那里收到的最后一次手动审查和批准，因此这是关键的一步。代码审查者评估代码的正确性，并评估更改是否可以安全地部署到生产环境中。他们评估代码是否有足够的测试(单元测试、集成测试和金丝雀测试)，是否为部署监视提供了足够的工具，以及是否可以安全地回滚。

虽然强制性提交前代码审查在许多组织中通常并不少见(事实上，这几乎是许多开放源码项目和组织的规范)，但我感到惊讶的原因是，我认为在将代码合并到主分支时，本文中详述的极其健壮的测试环境和基础设施会提供更大的自由度。

我们的结果表明，尽管驱动代码评审的最高动机仍然是发现缺陷，但实践和实际结果中发现错误的内容比预期的要少：与缺陷相关的评论所占比例很小，并且主要涵盖小的逻辑低层问题。同时，代码审查还为软件团队提供了广泛的好处，例如知识传递、团队意识和改进的问题解决方案。此外，我们发现对背景和变化的理解是任何审查的关键。

虽然有研究估计有效的代码审查可以检测到大约90%的错误，但强制性的提交前代码审查几乎总是阻碍开发人员的速度。

在最好的情况下，拉流请求在提交后几秒钟内就可以合并到主线上。这要求：

但更常见的是，CI和代码审查阶段都需要几分钟到几个小时才能完成，这需要开发人员等待那么长时间才能合并他们的拉请求。对于较大的特性分支来说尤其如此，尽管有时，即使是相对较小的补丁也可能因为许多不同的原因而陷入代码审查的泥潭数日之久(代码所有者可能正在度假，所讨论的项目可能是完全由志愿者驱动的开源项目，审查者和被审阅者之间可能会有大量的更改来回，等等)。

平均而言，我认为代码审查会将开发人员的速度从几分钟降低到几个小时，在最坏的情况下，会降低几天甚至几个月的速度(当更改导致需要开发人员多次离开主分支以避免合并冲突的长时间拉入请求时)。

提交前审查的另一种选择是提交后审查，它允许开发人员将更改合并到主干，并在后续的拉请求中处理审阅者的评论。

在许多方面，提交后审查提供了两全其美的好处：开发人员的速度不会牺牲在等待批准的祭坛上，合理的关注在后续提交中很快就会得到解决。

虽然提交后审查有许多警告(在本文的以下部分中有详细说明)，但主要支持提交后审查可以允许开发人员快速迭代他们正在开发的功能，同时保持较小的更改。

提交后审查自动意味着部署后审查，这是一个普遍存在的误解，即代码在部署后进行审查。这不一定是事实。

提交后审查提供了比人们想象的更大的灵活性。“提交后”审查可能意味着：

让代码审查成为部署管道的最后一步，然后再将代码部署到生产环境中

或者在不需要审核跟踪或强制签署更改的情况下(对于原型项目、不处理客户敏感数据的内部工具等)，在部署后进行代码审查。

能够快速连续地合并拉请求确保了开发人员可以更快地迭代他们正在开发的功能。它还鼓励开发人员保持较小的拉请求，确保在许多小的拉请求中开发特性(或者至少由许多原子提交组成)。

提交后审查的另一个好处是关注审阅者。在拉请求小而多的文化中，每隔10或15分钟就会被ping一次以进行审查，这可能会阻碍审阅者和被审阅者的工作效率。

转移到提交后审查格式使开发人员不需要对每个拉请求进行审批。它还允许审查者批量审查，因此他们可以一下子审查许多更改，而不是不断地被代码审查打断。此外，这激励审查者只对引起关注或有逻辑错误的合并拉请求发表评论。根据我自己的经验，这减少了评审者的吹毛求疵，如果评审者确实吹毛求疵，它允许开发人员从容不迫地解决次要问题。

提交后的审查鼓励更好的开发实践，这一事实是有道理的。

要达到提交后评审成为现实的程度，需要强大的文化支撑。至少，它需要：

在通过设计文档实现代码之前，在API设计等方面进行协作的文化。

如果设计文档在代码实现之前已经被清楚地充实和同意，那么实现将不会给审查者带来任何重大的意外。这反过来最小化(甚至消除了)由于审阅者和被审阅者之间特定于API设计或实现的分歧而导致的恢复提交的需要。

为了使提交后审查真正有效，将与代码样式或习惯用法相关的代码审查注释减到最少也很重要。可以说，这消除了审查者对代码审查的自动化方面进行评论的需要。

人们可以争辩说，无论如何，所有这些都是良好开发文化的标志(对于许多项目来说，这些都已经作为提交前CI工作流的一部分实现了)。提交后的审查使它们不可避免地只起到促进更好的开发实践的强制作用。

通常，只有在所有单元测试都通过后，代码才会合并到主干中。提交后审查与提交前审查没有什么不同。

之所以需要“提交后”测试，是因为单元测试(虽然在防止许多形式的严重故障方面非常有效)、LINT测试等不足以检测某些类别的错误，这些错误仅在针对真实依赖项进行测试时才出现在真实环境中。

通过“提交后”测试，我指的是代码更改在提交后但在部署前所经历的所有测试：集成测试、负载测试、浸泡测试、影子测试、异常跟踪等等。

为什么大多数“提交后”测试都需要在代码审查之后进行，这是没有道理的。足够复杂的“提交后”测试更有可能检测到更多的bug并提醒开发人员注意bug(或者更好的是，在源代码控制中自动恢复提交)，从而允许编写代码的开发人员在审查者甚至还没有查看代码之前就了解bug的性质并修复它。

这意味着审查者可以相当程度地确信更改在功能上是正确的，并且在审阅它之前经过了广泛的测试。然后，审查者可以真正专注于尝试查找即使是最高级形式的“提交后”测试也没有标记的错误类别：安全错误、任何测试都没有捕获的业务逻辑实现中的错误等等。

甚至可能有这样的场景，在将更改部署到生产之前，代码审查员不需要对其进行签字。对于大多数原型项目、没有遵从性要求的项目、不处理客户数据或敏感数据的项目等等，都是如此。在这种情况下，“提交后”代码审查甚至可以是“部署后”代码审查，即代码只有在部署到生产环境后才会进行审查。Quora是一个实施提交后、部署后审查的公司的具体例子。这篇博客文章详细介绍了Quora的提交后审查方法。

不用说，提交后评审在团队成员之间高度信任的团队中效果最好。建立信任和学习如何在团队中很好地工作需要时间。它还对如何让新的团队成员或初级工程师上岗提出了独特的挑战，他们可能对问题域、编程语言、框架、工具、测试指南甚至公司的CI/CD基础设施不够熟悉，无法立即适应提交后审查文化。

在这种情况下，从“提交前”评审开始让新的团队成员上岗可能会有所帮助，并朝着让团队成员足够快的速度逐步切换到提交后评审的目标努力。开放源码项目可以在“混合模式”下工作，在这种模式下，项目的维护者可以依赖提交后审查，而新的贡献者可以通过提供提交前审查来加入。获得项目维护性的一部分可能涉及从要求提交前审查到要求提交后审查的过程。

培养成功的“提交后”评审文化的下一个要求是对自动化和监控基础设施进行大量投资。如果提交被证明是有问题的，那么要求开发人员手动恢复更改并解决合并冲突可能会抵消提交后审查文化首先带来的几乎所有开发人员生产力方面的收益。恢复提交需要自动化，这需要测试基础设施和源代码控制管理(SCM)工具之间的紧密集成。

可以说，这在采用monorepo的组织中甚至更难实现，因为还原单个违规提交可能需要在整个代码库中还原数十个或数百个提交。大多数采用单一采购的组织都有广泛的自动化和工具来实现这一目的。一个例子是优步如何使用一个名为SubmitQueue的系统来始终保持主分支机构的规模绿色。简而言之，SubmitQueue是一个元构建系统，集负载平衡器和投机性执行器于一身，并加入了一定数量的数据科学，当可能有数百个并发提交时，它通过对主分支的提交的可串行化来保证“规模上始终是绿色的主分支”。

根据我自己的经验，在非monorepo环境中管理提交变得容易得多，因为恢复提交所需的自动化不需要研究论文级别的复杂性。

在开发人员生产力和高质量代码之间走钢丝总是一项具有挑战性的任务，需要明智的选择和权衡。开发人员的迭代速度可以通过提交后评审的任何一种方式来提高，即，

与所有好的事情一样，这需要时间和投资才能正确，但对于试图提高开发人员生产力的团队或组织来说，这肯定是一个值得探索的途径。