第九章 代码审查

代码审查是其他人审查代码的过程,通常发生在代码合入代码库之前。尽管代码审查的定义很简单,但它的过程实现在整个软件行业中存在很大差异。一些组织有一组精选的“守门员”来审查对代码库的更改,另外一些组织将代码审查流程交给较小的团队负责,允许不同的团队使用不同级别的代码审查。在谷歌,每个变更在提交之前都必须要经过审查,每个工程师都有责任发起审查和审查其他人发起的变更。

代码审查通常需要一个流程和支持该流程的工具。在Google,我们使用自定义代码审查工具Critique来支持我们的流程。Critique在Google 是一个相当重要的工具,以致于这本书会专门用一章来讲述它。本章通过在谷歌的实践代码重点介绍审查的过程,而不是特定的工具,因为这些基础知识比工具更古老,而且因为这些见解中的大部分都可以适用于你的任何代码审查工具。

代码审查的一些好处已经众所周知并且比较明显,比如在代码进入代码库前检测代码中的错误。不过,还有一些比较微妙的其他好处。由于Google的代码审查过程如此普遍和广泛,我们已经注意到这些更微妙的影响,包括心理影响,随着时间和规模的发展,这些心理影响为组织提供了许多助力。

代码审查流程

代码审查可以发生在软件开发的许多阶段。在Google,代码审查发生在将更改提交到代码库之前;这个阶段也称为提交前审查。代码审查的主要最终目标是获得另一位工程师同意此更改,届时我们将此更改标记为“对我来说看起来不错”(LGTM)来表示。我们将此LGTM当做一个必要的权限“位”(与下面提到的其他位组合)以允许提交更改。 在Google,一个典型的代码审查会经过以下步骤:

  1. 用户在其工作区中对代码库进行更改。接着该作者创建更改的快照:上传到代码审查工具的补丁和相应的描述。此更改会产生与代码库之间的差异,用于评估更改了哪些代码。
  2. 作者可以使用这个初始补丁来推进自动审阅或进行自我审阅。当作者对更改的差异感到满意时,他们会将更改传递给一个或多个审阅者。这个过程会通知这些审阅者,要求他们查看和评论快照。
  3. 审阅者在代码审查工具中打开更改并在差异上发表评论。一些评论会给出明确的解决方法,而另外一些只是提供信息。
  4. 作者根据反馈意见修改更改并上传新快照,然后回复给审阅者。步骤3和4可以重复多次。
  5. 审阅者对更改的最新状态感到满意后,他们同意更改并通过将其标记为“对我来说看起来不错”(LGTM)来接受它。默认情况下只需要一个LGTM,尽管惯例可能会要求所有审阅者同意更改。
  6. 更改标记为LGTM后,作者可以将更改提交到代码库,前提是他们解决了所有评论并且更改被审阅通过。我们将在下一节中介绍审阅通过。

代码是一种责任

要记住并认同代码本身是一种责任,而且可能是一种必要的责任。 但就其自身而言,代码只是对某人某处的维护任务。就像飞机携带的燃料一样,它也有重量,当然这对于飞机飞行来说是必要的。 当然,新功能通常是必要的,但在开发代码之前,首先要小心,以确保任何新功能都得到保证。重复的代码不仅是一种浪费,而且事实上它会比根本没有代码要花费更多的时间;代码库中不存在重复时,修改起来会很轻松,可一旦代码库存在重复,基于它的更改通常需要更多的努力。编写全新的代码是如此的令人沮丧,以至于我们中的一些人常说:“如果你从头开始编写它,那你就错了!”
对于库或工具代码更是如此。很有可能会出现以下场景,如果你正在编写一个工具代码,那么像Google大小的代码库中可能存在类似的代码。因此,第17章中讨论的工具对于查找此类工具代码和防止引入重复代码至关重要。理想情况下,这项研究是事先完成的,并且在编写任何新代码之前,已将任何新设计传达给相关的小组。
当然,一定会出现新项目、引入新技术、需要新组件等等。尽管如此,代码审查并不要重新讨论或辩论以前的设计决策。设计决策通常需要时间,它需要设计提案的传递、在API审查或类似会议中对设计的辩论,以及原型的开发。尽管对全新代码的代码审查不应该是一蹴而就的,代码审查过程本身也不应该被视为重新审视以前决策的机会。

谷歌是怎么做代码审查的

我们已经粗略地指出了典型的代码审查过程是如何工作的,但关键在于细节。 本节会详细讲述Google是如何做代码审查的,以及这些做法是如何使代码审查能够经得起时间的考验的。 要审核Google的任何更改,需要从三个方面“批准”: • 另一位工程师批准代码的正确性和理解性,确认该代码是合适的,并且符合作者表述的要求。这通常是团队成员,也可以不是团队成员。当代码同行评审员同意代码“看起来不错”(LGTM)后,便通过正确性和理解性检查。 • 代码所有者之一批准代码适用于代码库的某一特定部分(并且可以提交到特定目录)。如果作者就是所有者,则这种批准是绝对的。 Google的代码库是一个树结构,分层所有者具有特定目录的所有权。(见第 16 章)。 所有者充当其特定目录的看门人。任何工程师都可能提出更改,任何其他工程师都可能提出LGTM,但相关目录的所有者也必须批准将此添加到他们的代码库中。这样的所有者可能是技术负责人或其他被认为是代码库特定领域专家的工程师。通常由每个团队决定分配所有权特权的范围。
• 由可读性审查者批准代码符合语言的风格和最佳实践,检查代码是否以我们期望的方式编写。如果作者是可读性审查者,那么这种批准同样是绝对的。这些工程师来自全公司范围内的工程师,他们被授予该编程语言的可读性审查者。

尽管这种粒度的控制听起来很繁重,而且,不可否认,有时确实如此, 大多数批准都是由一个人承担所有三个角色,这大大加快了批准的过程。重要的是,作者也可以承担后两个角色,只需要来自另一位工程师的LGTM就将代码合并到他们自己的代码库中,前提是他们已经具有该语言的可读性(所有者经常这样做)。

这样的话,代码审查过程就非常灵活了。作为项目所有者并具有该代码语言可读性的技术负责人,可以提交一份 仅有另一位工程师的LGTM的代码。没有这种权限的实习生可以向同一个代码库提交相同的更改,前提是他们获得了具有语言可读性的所有者的批准。上述三个批准没有顺序要求。作者甚至可以明确标记出来,希望获得所有审阅者的LGTM,而不再只是一个LGTM。

在实践中,大多数需要多次批准的代码审查通常会经历两个步骤:从同行工程师那里获得LGTM,然后寻求适当的代码所有者/可读性审查者的批准。这允许两个角色专注于代码审查的不同方面并节省审查时间。初级审查者可以关注代码的正确性和代码更改的总体有效性;代码所有者可以专注于此更改是否适合他们的代码库部分,而不必关注每一行代码的细节。换句话说,审批者经常寻找与同行评审者不同的东西。毕竟,有人试图将代码签入到他们的项目/目录中。他们更关心这样的问题:“这段代码是容易维护还是难以维护?”,“它会增加我的技术债务吗?”,“我们团队中是否有专业知识来维护这些代码”。

如果所有这三种类型的审查都可以由一个审查者处理,为什么不让这些类型的审查者处理所有代码审查?简短的回答是因为规模。分离这三个角色能够增加了代码审查过程的灵活性。如果你正在与同行合作开发新的工具代码,你可以让团队中的某个人审查代码的正确性和可理解性。经过几轮(也许几天内)后,你的代码满足你的同行评审者的要求,你将获得LGTM。 这样的话,你只需要获得库的所有者(并且所有者通常具有适当的可读性)来批准就可以了。

代码审查的好处

在整个行业中,代码审查虽然并没有形成一个统一的实践,但是代码审查本身并没有争议。许多(甚至可能是大多数)其他公司和开源项目都有某种形式的代码审查,并且这其中的大多数都认为,代码审查与将新代码引入代码库的完整性检查一样重要。软件工程师了解代码审查的一些更明显的好处,即使他们个人可能并不认为它适用于所有情况。但在谷歌,代码审查通常比大多数其他公司执行得更彻底和更广泛。

与许多软件公司的文化一样,谷歌文化的基础是给予工程师宽松的工作自由度。有证据表明,严格的流程往往不适用于需要快速跟进新技术的充满活力的公司,而官僚制度往往不适用于专业的创意人才。 然而,代码审查是一项强制要求,是Google的所有软件工程师都必须参与的为数不多的一些流程之一。Google要求对代码库的几乎所有代码更改进行代码审查,无论是多小的更改。这个确实会有成本,也会影响工程速度,因为它确实会减慢将新代码合入代码库的速度,同时影响代码上线的时间。(这两个都是软件工程师对严格的代码审查过程的常见抱怨。) 那么,为什么我们需要这个过程?为什么我们认为这是一项长期利益?

精心设计的代码审查流程和认真对待代码审查的文化会带来以下好处: • 检查代码正确性 • 确保其他工程师可以理解更改的代码 • 强制跨代码库的一致性 • 从心理上提升团队所有权 • 实现知识共享 • 提供代码审查的历史记录 随着时间的推移,其中许多好处对软件组织至关重要,其中许多好处不仅对作者有益,而且对审阅者也有益。以下部分将详细介绍这些项目中的每一个。

代码的正确性

代码审查的一个明显好处是它允许审查者检查代码更改的“正确性”,让另一双眼睛来查看代码更改更有助于确保更改符合预期。审阅者通常会查看更改是否经过适当的测试、设计是否正确以及是否正确有效地运行。在许多情况下,检查代码正确性是检查特定更改是否会将错误引入代码库。

许多报告都指出代码审查在预防软件后期的Bug方面的功效。IBM的一项研究发现,不出所料,在流程中更早地发现缺陷可以减少以后修复它们所需的时间。对代码审查投入的时间会节省花在测试、调试和回归测试的时间,前提是代码审查过程本身经过简化以保持轻量级。后一点很重要,重量级的或无法正确扩展的代码审查流程会变得不可持续。 本章后面,我们将介绍一些保持流程轻量级的最佳实践。

为了让对代码正确性的评估变得更客观而不是更主观,无论是在设计中还是在引入更改的功能中,作者通常会尊重他们的特定方法。审阅者不应该因为个人意见而提出替代方案,审阅者可以提出替代方案,但前提是他们提高代码理解性(例如通过降低复杂性)或功能性(例如通过提高效率)。一般来说,鼓励工程师批准改进代码库的更改,而不是等待为一个“完美”的解决方案达成共识。这种关注往往会加快代码审查。

随着工具变得更强大,许多正确性检查是通过诸如静态分析和自动化测试之类的技术自动执行的(尽管工具可能永远不会完全消除人工检查代码的价值——有关更多信息,请参阅第20章)。尽管工具有其局限性,但它确实告诫了我们有必要依靠人工代码审查来检查代码正确性。

也就是说,在初始代码审查过程中检查缺陷仍然是一个通用“前移”策略的一个组成部分,旨在尽早发现和解决问题,这样它们就不需要在开发周期中进一步增加成本和资源。代码审查既不是万能药,也不是检查此类正确性的唯一方法,但它是针对软件中此类问题的纵深防御的一个要素。因此,代码审查不需要“完美”才能获得结果。

令人惊讶的是,检查代码正确性并不是Google从代码审查过程中获得的主要好处。检查代码正确性通常确保更改有效,但更重要的是确保在随着时间的推移和代码库本身的扩展中,代码的变化是可以理解的并且是有意义的。为了评估这些,除代码是否简单地逻辑正确或易懂之外,我们还要确认其他因素。

代码的可理解性

代码审查通常是除作者之外的其他人检查更改的第一个机会, 这就允许审阅者做一些即使是最好的工程师也做不到的事情:提供不偏不倚的反馈。代码审查通常是对更改是否能被更广泛的受众理解的第一个测试。这种观点非常重要,因为代码的阅读次数比编写的次数要多得多,并且理解至关重要。

通常情况下,与作者岗位职责不同的审阅者很有用,尤其在工作中需要维护或使用待评审代码的审阅者。此时,不需要谦虚谨慎地提意见,在处理有关代码可理解性方面,坚信“客户永远是对的”。在某些方面,你现在得到的任何问题都会随着时间的推移成倍增加,因此不能忽视有关代码可理解性的每个问题。这并不意味着你需要改变代码方法或逻辑来回应批评,但这确实意味着你可能需要更清楚地解释代码。

总之,代码正确性和代码可理解性是来自另一位工程师的LGTM的主要标准,这是代码审查的指标之一。当工程师将代码标记为LGTM时,就表示他们认为代码按照它所说的去做并且可以理解。不过,除此之外,Google还要求可持续维护代码,因此在某些情况下,我们需要对代码进行其他维度的审批。