Skip to content

CHAPTER 9 Code Review

kimi edited this page Dec 15, 2021 · 7 revisions

代码审查

代码审查是一个由作者以外的人对代码进行审查的过程,通常在将该代码引入代码库之前。虽然这是一个简单的定义,但在整个软件行业中,代码审查过程的实施有很大不同。一些组织在代码库中拥有一组精选的 "守门人"来审查修改。其他组织将代码审查过程委托给较小的团队,允许不同的团队要求不同级别的代码审查。在谷歌,基本上每一个变化在提交之前都会被审查,每个工程师都负责启动审查和审查变更。

代码审查通常需要一个流程和支持该流程的工具的组合。在谷歌,我们使用一个定制的代码审查工具,Critique,来支持我们的流程。Critique在Google中是一个非常重要的工具,以至于在本书中应该有自己的一章。这一章的重点是Google采用的代码审查流程,而不是具体的工具,这是因为这些基础比工具更古老,而且这些见解大多可以适用于你可能使用的任何代码审查的工具。

关于 Critique 的更多信息,见第19章。

代码审查的一些好处,公认且明显的好处例如在代码进入代码库之前检测到代码中的错误。然而,其他的好处则更为微妙。由于谷歌的代码审查过程是如此的普遍和广泛,我们已经注意到了许多更微妙的影响,包括心理上的影响,随着时间的推移和规模的扩大,会给一个组织带来许多好处。

代码审查流程

代码审查可以发生在软件开发的许多阶段。在谷歌,代码审查是在一个改动可以提交到代码库之前进行的;这个阶段也被称为提交前审查。代码审查的主要最终目标是让另一位工程师同意该变更,我们将该变更标记为 "对我来说看起来不错"(LGTM, looks good to me)。我们把这个LGTM作为一个必要的权限 "位"(结合下面提到的其他位),以允许修改被提交。

在谷歌,一个典型的代码审查经历了以下步骤。

  1. 一个用户在他们的工作区对代码库写了一个改动。然后,该作者创建一个变更的快照:一个补丁和相应的描述,并上传到代码审查工具。这个变化产生了与代码库的差异,用来评估哪些代码发生了变化。
  2. 作者可以使用这个初始补丁来应用自动审查意见或进行自我审查。当作者对修改的差异感到满意时,他们将修改的内容邮寄给一个或多个审查者。这个过程会通知这些审查员,要求他们查看和评论快照。
  3. 评审员在代码评审工具中打开修改,并在差异上发表评论。有些评论要求明确的解决。有些仅仅是信息性的。
  4. 作者修改变更,并根据反馈上传新的快照,然后回复给审查者。步骤3和步骤4可以重复多次。
  5. 在审查者对修改的最新状态感到满意后,他们同意该修改,并将其标记为 "对我来说很好"(LGTM),从而接受它。默认情况下,只需要一个LGTM,尽管惯例可能要求所有的评审员都同意该修改。
  6. 当一个变更被标记为LGTM后,作者可以将该变更提交到代码库中,前提是他们解决了所有的评论,并且该变更被批准。我们将在下一节讨论批准问题。

我们将在本章后面更详细地介绍这一过程。

代码是一种责任
记住(并接受),代码本身是一种责任,这一点很重要。它可能是一种必要的责任,但就其本身而言,代码对某个人来说只是一项维护任务。就像飞机携带的燃料一样,它也有重量,当然,它是飞机飞行的必要条件。
当然,新的功能往往是必要的,但在开发代码之前,首先要注意确保任何新的功能是有必要的。重复的代码不仅是一种浪费,它实际上会比没有代码花费更多的时间;当代码库中存在重复时,在一种代码模式下可以很容易地进行的改变往往需要更多的努力。编写全新的代码是非常不受欢迎的,以至于我们中的一些人有一个说法。"如果你从头开始写,你就做错了!"
这对库或实用程序代码来说尤其如此。有可能,如果你正在写一个实用程序,那么在像Google那样大的代码库中,其他人可能也做过类似的事情。因此,像第17章中讨论的那些工具,对于找到这些实用程序代码和防止引入重复的代码都是至关重要的。理想情况下,这种研究是事先完成的,在编写任何新的代码之前,任何新的设计都已经传达给适当的小组。
当然,新项目的发生,新技术的引入,新组件的需要,等等。综上所述,代码审查并不是一个重提或辩论以前设计决定的场合。设计决策通常需要时间,需要分发设计方案,在API评审或类似的会议上对设计进行辩论,也许还需要开发原型。就像对全新的代码进行代码审查不应该突然出现一样,代码审查过程本身也不应该被看作是重新审视以前决定的机会。

谷歌是如何审查代码的

我们已经大致指出了典型的代码审查过程是如何运作的,但魔鬼就在细节中。本节详细介绍了代码审查在谷歌是如何运作的,以及这些做法是如何让它随着时间的推移而适当扩展的。

在谷歌,任何特定的变更有三个方面的审查需要被"批准"。

  • 另一位工程师对代码的正确性和理解力进行检查,以确保代码是合适的,并能做到作者所声称的那样。这通常是一个团队成员,尽管不一定是。这反映在LGTM权限的 "位"上,在同行评审员同意代码 "看起来不错"之后,将设置该位。
  • 来自代码所有者之一的批准,该代码适合于代码库的这个特定部分(并且可以被签入到一个特定的目录)。如果作者是这样一个所有者,这种批准可能是隐含的。谷歌的代码库是一个树状结构,有特定目录的分层所有者。(见第16章) 所有者作为他们特定目录的看门人。任何工程师都可以提出修改建议,任何其他工程师也可以提出LGTM,但有关目录的所有者必须批准在他们的代码库中增加这一内容。这样的所有者可能是技术领导或其他被认为是代码库特定领域的专家的工程师。一般来说,由每个团队决定如何广泛或狭窄地分配所有权权限。
  • 由具有语言 "可读性"的人批准代码符合语言的风格和最佳实践,检查代码是否以我们期望的方式编写。如果作者具有这种可读性,这种批准也可能是隐含的。这些工程师是从全公司范围内被授予该编程语言可读性的工程师库中抽取的。

虽然这种程度的控制听起来很繁琐--而且,不得不承认,有时确实如此--但大多数审查都是由一个人承担这三个角色,这就大大加快了审查过程。重要的是,作者也可以承担后两个角色,只需要另一个工程师的LGTM就可以将代码签入到他们自己的代码库中,前提是他们已经具备了该语言的可读性(所有者往往如此)。

这些要求使代码审查过程变得相当灵活。作为项目所有者的技术带头人,如果拥有该代码的语言可读性,可以只用另一个工程师的LGTM来提交代码修改。一个没有这种权力的实习生可以对同一个代码库提交同样的修改,只要他们得到拥有语言可读性的所有者的批准。上述三个许可 "位"可以任意组合。作者甚至可以向不同的人申请一个以上的LGTM,方法是明确地将该修改标记为需要所有审查者的LGTM。

在实践中,大多数需要一个以上批准的代码审查通常要经过两个步骤:从同行工程师那里获得LGTM,然后再从适当的代码所有者/可读性审查者那里寻求批准。这使得这两个角色可以专注于代码审查的不同方面,并节省审查时间。主要的审查者可以专注于代码的正确性和代码修改的一般有效性;代码所有者可以专注于这个修改是否适合他们的代码库的一部分,而不必专注于每一行代码的细节。换句话说,审批者所寻找的东西往往与同行评审者不同。毕竟,有人是想把代码签入他们的项目/目录。他们更关心的是诸如以下问题。"这段代码是容易还是难以维护?" "它是否增加了我的技术债务?" "我们的团队中是否有维护它的专业知识?"

如果这三种类型的审查都可以由一个审查员来处理,为什么不直接让这些类型的审查员来处理所有的代码审查?简短的答案是规模。将这三种角色分开,可以增加代码审查过程的灵活性。如果你和同行一起在一个实用程序库中开发一个新的函数,你可以让你团队中的某人来审查代码的正确性和理解性。经过几轮审查(可能是几天),你的代码让你的同行审查员满意了,你就得到了一个LGTM。现在,你只需要让库的所有者(而所有者往往具有适当的可读性)批准这一修改。

所有权
当一个小团队在专门的版本库中工作时,通常会授予整个团队对版本库中所有内容的访问权。毕竟,你认识其他的工程师,这个领域很窄,你们每个人都可以成为专家,而且人数少,限制了潜在错误的影响。
随着团队规模的扩大,这种方法可能无法扩展。其结果是,要么是混乱的版本库分裂,要么是用不同的方法来记录谁在版本库的不同部分拥有什么知识和责任。在谷歌,我们称这套知识和责任为所有权,而行使这些知识和责任的人为所有者。这个概念不同于对源代码集合的占有,而是意味着一种管理意识,以公司的最佳利益对代码库的某个部分采取行动。(事实上,如果我们再来一次,"管家"几乎肯定是一个更好的术语。)
特别命名的OWNERS文件列出了对一个目录及其子目录有所有权责任的人的用户名。这些文件也可能包含对其他OWNERS文件或外部访问控制列表的引用,但最终它们会解析为一个个人列表。每个子目录也可能包含一个单独的OWNERS文件,而且这种关系是分层递增的:一个给定的文件通常由目录树中它上面的所有OWNERS文件的成员联合拥有。OWNERS文件可以有任意多的条目,但我们鼓励一个相对较小和集中的列表,以确保责任明确。
对谷歌代码的所有权传达了对其权限范围内的代码的批准权,但这些权利也伴随着一系列的责任,如了解所拥有的代码或知道如何找到拥有这些代码的人。不同的团队在授予新成员所有权方面有不同的标准,但我们一般鼓励他们不要把所有权作为入职仪式,并鼓励离开的成员在可行的情况下尽快放弃所有权。
这种分布式所有权结构使我们在本书中提出的许多其他做法成为可能。例如,根OWNERS文件中的一组人可以作为大规模修改的全球批准者(见第22章),而不必打扰本地团队。同样,OWNERS文件作为一种文档,使人们和工具很容易找到对某段代码负责的人,只要沿着目录树走就可以了。当新项目被创建时,没有中央机构需要注册新的所有权权限:一个新的OWNERS文件就足够了。
这种所有权机制简单而强大,在过去20年中得到了良好的扩展。这是谷歌确保数以万计的工程师能够在单一资源库的数十亿行代码上高效运作的方法之一。

代码审查的好处

在整个行业中,代码审查本身并不存在争议,尽管它远不是一种普遍做法。许多(甚至可能是大多数)其他公司和开源项目都有某种形式的代码审查,而且大多数人认为这个过程很重要,是对引入新代码到代码库的理智检查。软件工程师理解代码审查的一些更明显的好处,即使他们个人可能不认为它适用于所有情况。但在谷歌,这个过程通常比其他大多数公司更彻底、更广泛。

谷歌的文化,就像很多软件公司的文化一样,是基于给工程师们广泛的自由度来完成他们的工作。人们认识到,对于一个需要对新技术做出快速反应的动态公司来说,严格的程序往往不能很好地工作,而官僚主义的规则往往不能很好地与创造性的专业人士合作。然而,代码审查是一项任务,是谷歌所有软件工程师都必须参与的少数全面流程之一。谷歌要求对代码库的每一次代码修改都要进行代码审查,无论多么微小。这个任务确实对工程速度有成本和影响,因为它确实减慢了将新代码引入代码库的速度,并可能影响任何特定代码变更的生产时间。(这两点是软件工程师对严格的代码审查过程的普遍抱怨。) 那么,为什么我们要要求这个过程?为什么我们相信这是一个长期的好处?

一个精心设计的代码审查过程和认真对待代码审查的文化会带来以下好处。

  • 检查代码的正确性
  • 确保其他工程师能够理解代码的变化
  • 强化整个代码库的一致性
  • 从心理上促进团队的所有权
  • 实现知识共享
  • 提供代码审查本身的历史记录

这些好处中,有许多对软件组织来说是至关重要的,而且其中有许多不仅对作者有利,而且对审查者也有利。下面的章节将对这些项目中的每一项进行更详细的说明。

代码的正确性

代码审查的一个明显的好处是,它允许审查者检查代码修改的 "正确性"。让另一双眼睛来检查一个变化,有助于确保这个变化能达到预期的效果。评审员通常会检查一个变化是否有适当的测试,是否有适当的设计,以及功能是否正确和有效。在许多情况下,检查代码的正确性就是检查特定的变化是否会在代码库中引入错误。

许多报告指出了代码审查在防止软件未来出现错误方面的功效。IBM的一项研究发现,在一个过程中更早地发现缺陷,不出所料,会导致后来修复缺陷所需的时间减少。对代码审查时间的投资节省了原本用于测试、调试和执行回归的时间,前提是代码审查过程本身被精简以保持其轻量。后面这一点很重要;代码审查过程如果过于沉重,或者不能适当扩展,就会变得不可持续。

为了防止对正确性的评价变得更加主观而不是客观,一般都会尊重作者的特定方法,无论是在设计上还是在引入的功能上的改变。评审员不应该因为个人观点而提出替代方案。审稿人可以提出替代方案,但前提是这些替代方案能够改善理解性(例如,通过减少复杂性)或功能性(例如,通过更有效)。一般来说,我们鼓励工程师批准那些能够改善代码库的修改,而不是等待对一个更 "完美"的解决方案达成共识。这种关注往往会加快代码审查的速度。

随着工具化的加强,许多正确性检查通过静态分析和自动测试等技术自动进行(尽管工具化可能永远不会完全消除基于人的代码检查的价值--更多信息见第20章)。尽管这种工具有它的局限性,但它无疑减少了依赖基于人的代码审查来检查代码正确性的需要。

也就是说,在最初的代码审查过程中检查缺陷仍然是一般的 "左移"策略的一个组成部分,目的是在尽可能早的时间发现并解决问题,这样就不需要在开发周期的更远处增加成本和资源。代码审查既不是万能的,也不是检查这种正确性的唯一方法,但它是对软件中这种问题的深入防御的一个要素。因此,代码审查不需要 "完美"来达到效果。

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

对代码的理解

代码审查通常是作者以外的人检查修改的第一个机会。这种视角使审查者能够做到最好的工程师也做不到的事情:提供不受作者视角影响的反馈。代码审查通常是对一个特定的变化是否能被更多人理解的第一个测试。这种观点是非常重要的,因为代码被阅读的次数要比它被写的次数多,理解和领悟是非常重要的。

找到一个与作者观点不同的评审员通常是很有用的,特别是一个评审员,作为他们工作的一部分,可能需要维护或使用修改中提出的代码。与审查员在设计决策方面应该给予作者的尊重不同,用 "客户永远是对的" 这一格言来对待关于代码理解的问题往往是有用的。在某种程度上,你现在收到的任何问题都会随着时间的推移而成倍增加,所以要把每个关于代码理解的问题看作是有效的。这并不意味着你需要改变你的方法或逻辑来回应批评,但它确实意味着你可能需要更清楚地解释它。

代码正确性和代码理解力的检查共同构成了另一个工程师的LGTM的主要标准,这也是一个被批准的代码审查所需的众多批准点之一。当一个工程师将代码审查标记为LGTM时,他们是在说,代码做了它所说的事情,而且它是可以理解的。然而,谷歌也要求代码是可持续维护的,所以我们在某些情况下对代码还需要额外的批准。

代码的一致性

在规模上,你写的代码会被别人依赖,并最终被别人维护。许多人需要阅读你的代码并理解你的工作。其他人(包括自动化工具)可能需要重构你的代码,直到你转到另一个项目。因此,代码需要符合一些一致性的标准,这样它才能被理解和维护。代码也应该避免过于复杂;简单的代码对其他人来说也更容易理解和维护。评审员可以在代码评审中评估这些代码是否符合代码库本身的标准。因此,代码审查应该采取行动,确保代码的健康。

正是为了可维护性,代码审查的LGTM状态(表示代码的正确性和理解力)与可读性批准的状态是分开的。可读性的批准只能由成功通过特定编程语言的代码可读性培训过程的人授予。例如,Java代码需要有 "Java可读性" 的工程师来批准。

可读性批准者的任务是审查代码,以确保它遵循该特定编程语言的最佳实践,与谷歌代码库中该语言的代码库一致,并避免过于复杂。一致和简单的代码更容易理解,当需要重构时,工具也更容易更新,使其更有弹性。如果一个特定的模式在代码库中总是以一种方式完成,那么写一个工具来重构它就会更容易。

此外,代码可能只写一次,但它会被阅读几十次、几百次,甚至几千次。在整个代码库中拥有一致的代码可以提高所有工程人员的理解力,这种一致性甚至影响到代码审查过程本身。一致性有时会与功能性发生冲突;一个可读性审查员可能更喜欢一个不太复杂的变化,它可能在功能上没有 "更好",但更容易理解。

有了一个更一致的代码库,工程师就更容易介入并审查别人项目的代码。工程师们可能偶尔需要向团队外寻求代码审查方面的帮助。能够伸出援手,请专家来审查代码,知道他们可以期望代码本身是一致的,使这些工程师能够更正确地关注代码的正确性和理解力。

心理和文化上的好处

代码审查也有重要的文化好处:它向软件工程师们强调,代码不是 "他们的",实际上是集体企业的一部分。这种心理上的好处可能是微妙的,但仍然是重要的。如果没有代码审查,大多数工程师会自然地倾向于个人风格和自己的软件设计方法。代码审查过程迫使作者不仅要让别人提出意见,而且要为了更大的利益作出妥协。

人类的天性是以自己的手艺为荣,不愿意将自己的代码开放给别人批评。对于自己编写的代码的批评性反馈,也很自然地不愿意接受。代码审查过程提供了一个机制,以减轻可能是一个情绪化的互动。代码审查,当它工作得最好的时候,不仅对工程师的假设提出了挑战,而且还以一种规定的、中立的方式进行审查,以缓和任何可能针对作者的批评,如果是以不请自来的方式提供的话。毕竟,这个过程需要批判性的审查(事实上,我们把我们的代码审查工具称为 "批判"),所以你不能责怪审查者做他们的工作和批评。因此,代码审查过程本身可以充当 "坏警察",而审查者仍然可以被看作是 "好警察"。

当然,不是所有的,甚至是大多数的工程师都需要这样的心理设备。但是,通过代码审查的过程来缓冲这种批评,往往能为大多数工程师提供一个更温和的介绍,让他们了解团队的期望。许多加入谷歌的工程师,或者一个新的团队,都对代码审查感到恐惧。我们很容易认为任何形式的批评性审查都会对一个人的工作表现产生负面影响。但随着时间的推移,几乎所有的工程师都期望在发送代码审查时受到挑战,并开始重视通过这一过程提供的建议和问题(尽管,承认,这有时需要一段时间)。

代码审查的另一个心理上的好处是验证。即使是最有能力的工程师也会患有冒名顶替综合症,并且过于自我批判。像代码审查这样的过程是对一个人的工作的验证和认可。通常,这个过程涉及到思想交流和知识共享(在下一节中涉及),这对审查者和被审查者都有好处。随着工程师的领域知识的增长,他们有时很难得到关于如何改进的积极反馈。代码审查的过程可以提供这种机制。

启动代码审查的过程也迫使所有作者对他们的修改多加注意。许多软件工程师并不是完美主义者;大多数人都会承认,"能完成工作" 的代码要比完美但开发时间太长的代码要好。如果没有代码审查,我们中的许多人自然会偷工减料,甚至完全打算在以后纠正这种缺陷。"当然,我没有完成所有的单元测试,但我可以以后再做。" 代码审查迫使工程师在发送修改前解决这些问题。从心理上讲,为代码审查而收集修改的组成部分,迫使工程师确保他们所有的事情都是一帆风顺的。在发送修改前的那一小段思考时间,是阅读修改的最佳时机,以确保你没有遗漏任何东西。

知识共享

代码审查的一个最重要的,但被低估的好处是知识共享。大多数作者挑选的评审员都是专家,或者至少是有知识的,在被评审的领域。评审过程允许评审员向作者传授领域知识,允许评审员向作者提供建议、新技术或咨询信息。(审稿人甚至可以把一些评论标记为 "仅供参考",不需要采取任何行动;它们只是作为作者的一种帮助而加入。) 在代码库的某一领域变得特别精通的作者往往也会成为所有者,然后他们反过来能够作为其他工程师的审查员。

代码审查过程中的反馈和确认包括询问为什么要以特定方式进行修改。这种信息交流有利于知识共享。事实上,许多代码审查涉及到双向的信息交流:作者和审查者都可以从代码审查中学习新的技术和模式。在谷歌,审查员甚至可以在代码审查工具本身中直接与作者分享建议的编辑。

工程师可能不会阅读每一封发给他们的电子邮件,但他们往往会回复每一封发送的代码审查。这种知识共享也可以跨时区和项目进行,利用谷歌的规模将信息快速传播给代码库各个角落的工程师。代码审查是知识转移的最佳时机:它是及时和可操作的。(谷歌的许多工程师首先通过代码审查 "认识" 其他工程师!)。

鉴于谷歌工程师花在代码审查上的时间,所积累的知识是相当重要的。当然,谷歌工程师的主要任务仍然是编程,但他们的很大一部分时间仍然花在代码审查上。代码审查过程提供了软件工程师之间相互交流和交换编码技术信息的主要方式之一。通常情况下,新的模式是在代码审查的背景下宣传的,有时是通过大规模的修改等重构。

此外,由于每一个变化都会成为代码库的一部分,代码审查就像一个历史记录。任何工程师都可以检查谷歌的代码库,并确定何时引入某些特定的模式,并提出有关的实际代码审查。通常情况下,这种考古学为比原作者和审查者更多的工程师提供了洞察力。

代码审查最佳实践

诚然,代码审查会给一个组织带来摩擦和延迟。这些问题大多不是代码审查本身的问题,而是他们选择的代码审查的实施问题。在谷歌保持代码审查过程的顺利运行也不例外,它需要一些最佳实践,以确保代码审查值得在这个过程中投入精力。这些实践中的大多数都强调要保持流程的灵活性和快速性,以便代码审查可以适当地扩展。

Clone this wiki locally