代码评审标准

代码评审的主要目的是确保 Google 代码库的整体代码运行状况随着时间的推移而不断改善。代码评审的所有工具和过程都是为此设计的。

为了实现这一点，必须做出一系列的权衡。

首先，开发人员必须能够在其任务上取得进展。如果开发人员从未向代码库提交过改进，那么代码库将永远不会得到改善。另外，如果评审人员不进行任何更改，那么之后开发人员也没有动力进行改进。

另一方面，评审人员有责任确保每个 CL（变更列表）的质量，使得其代码库的整体代码运行状况不会随着时间的流逝而减少。这可能很棘手，因为随着时间的推移，代码库通常会由于代码运行状况的小幅下降而退化，尤其是在开发团队时间紧迫，认为必须采取捷径才能实现其目标的情况下。

此外，评审人员对他们正在评审的代码要负起责任。确保代码库保持一致性和可维护性，以及“代码评审中的查找内容”中提到的其他事项。

因此，我们将以下规则作为代码评审中期望的标准：

通常，即使 CL 并不完美，不过如果达到可以提升系统整体代码质量的程度，评审人员就可以批准它。

这是所有代码评审指南中的最高原则。

当然，是有例外的。例如，如果 CL 添加了评审人员不希望在其系统中使用的功能，那么即使代码设计得当，评审人员也可以肯定的拒绝批准。

这里的关键点是，没有“完美”的代码，只有更好的代码。评审人员不应要求开发人员在批准前抛光 CL 的每一小块细节。评审人员要追求的不应该是完美，而应该是持续改进。总体而言，如果一个 CL 可以提高系统的可维护性，可读性和可理解性，那就不应该因为它不是“完美的”而被延迟几天甚至几周。

评审人员应该随时提供建议，这样开发人员可能会做得更好，但是如果不是很重要的建议，可以在其前面加上“Nit：”之类的字眼，以使开发人员知道这只是一个改进建议，他们可以选择忽略。

指导

代码评审具有重要的功能，可以传授开发人员关于语言，框架或通用软件设计原理的新知识。随着时间的推移共享知识会成为改善系统代码运行质量的一部分。但要注意，如果你的建议纯粹是带有教育性质的，并且对于满足本文所描述的标准来说并不是那么重要，那么请在前面加上“Nit:”，或者以其他方式告诉开发人员，他们并不一定要在 CL 中解决这些问题。

原则

• 客观的技术和数据比个人意见和偏好更重要。
• 在代码风格方面，样式指南是绝对权威。不在样式指南中的任何纯样式点（空格等）都是个人喜好问题。样式应与那里的样式保持一致。如果没有以前的样式，请接受作者的样式。
• 软件设计方面从来都不是纯粹的样式问题，也不是个人喜好。它们基于基本原则，应权衡这些原则，而不仅仅是个人意见。有时有一些有效的选择。如果开发 人员可以证明（通过数据或基于可靠的工程原理）几种方法同样有效，那么评审人员应接受开发人员的选择。否则就应该基于软件设计标准原则做出决定。
• 如果没有其他的适用规则，那么评审人员可以要求开发人员与当前代码库中的内容保持一致，只要不会使系统的总体代码运行状况恶化就可以。

解决冲突

在代码评审中发生冲突时，第一步应始终是使开发人员和评审人员根据本档以及《CL 作者指南》和《审阅者指南》中其他文档的内容达成共识。

当达成共识变得特别困难时，让开发人员和评审人员进行面对面的会议或 VC 可能会有所帮助，而不仅仅是尝试通过代码评审注释来解决冲突。（但是，如果这样做，请确保将讨论的结果记录在 CL 上的注释中，以备将来阅读。）

如果还不能解决问题，那么就要考虑把问题升级。升级的途径通常是进行更广泛的团队讨论，其中包括让团队负责人参与进来，请求代码维护人员作出决定，或请求工程经理提供帮助。不要因为开发人员和评审人员无法达成一致意见就让 CL 一直挂在那里。

代码评审中的查找内容

设计

代码评审中最重要的部分是 CL 的总体设计。CL 中各种代码的交互是否有意义？此更改是属于代码库还是属于某个包？它与系统的其余部分是否集成良好？现在是添加此功能的好时机吗？

功能性

此 CL 是否达到开发人员的预期目的？开发人员打算为该代码的用户带来什么好处？“用户”通常既是最终用户（当他们受到更改影响时）又是开发人员（将来他们将不得不“使用”此代码）。

通常，我们希望开发人员能够对 CL 进行良好的测试，以确保它们在进行代码审查时能够正常工作。但是，作为评审人员，仍然应该考虑边缘情况，寻找并发性问题，尝试像用户一样思考，并找出仅仅通过阅读代码不能看到的错误。

您可以根据需要验证 CL，对于评审人员来说，检查 CL 的行为最好的时机是当它对用户产生了影响，例如 UI 更改。当只是阅读代码的时候，很难理解一些更改将如何影响用户。对于这样的更改，如果过于麻烦而无法在 CL 中打补丁并自己尝试，则可以让开发人员提供功能演示。

在代码评审期间考虑功能性的另一个时机点是，CL 中是否正在进行某种并行编程，从理论上讲可能导致死锁或竞争条件。仅通过运行代码很难检测到这类问题，通常需要有人（开发人员和评审人员）仔细考虑它们，以确保不会引入问题。

复杂性

CL 是否比实际需要的要复杂？在 CL 的每个级别都进行检查 —— 个别行是否太复杂？功能太复杂？类太复杂？“过于复杂”通常意味着“代码阅读器无法快速理解。”也可能意味着“开发人员在尝试调用或修改此代码时可能会引入错误。”

过度设计一种特殊的复杂性，即开发人员使代码变得比实际需要的通用，或者增加了系统目前不需要的功能。评审人员应特别警惕过度设计。鼓励开发人员解决他们现在需要解决的已知问题，而不是解决开发人员推测的将来可能需要解决的问题。未来的问题应该在它们出现之后再去解决，因为到了那个时候我们可以看到它们的实际状况和需求。

测验

根据更改要求进行单元测试，集成测试或端到端测试。通常，除非 CL 处理紧急情况，否则应在与生产代码相同的 CL 中添加测试。

确保 CL 中的测试正确，合理且有用。测试无法自我测试，我们很少为测试编写测试，所以必须确保测试有效。

代码破损时，测试会失败吗？如果代码在其下面更改，它们将开始产生误报吗？每个测试都会做出简单而有用的断言吗？测试是否在不同的测试方法之间适当地分开？

请记住，测试也是必须维护的代码。

命名

开发人员是否使用了良好的命名方式？好的命名要能够充分表达一个项（变量、类名等）是什么或者用来做什么，但又不会因为太长而难以阅读。

注释

开发人员是否用可理解的自然语言写下清晰的注释？所有注释都是必要的吗？通常，好的注释应该解释为什么存在某些代码，而不应该解释某些代码在做什么。如果代码不够清晰，无法解释自身，则应使简化代码。也有一些例外情况（例如，正则表达式和复杂算法通常会在注释中解释它们的作用，会让阅读代码的人受益匪浅），但大多数注释是针对代码本身可能无法包含的信息，比如这些代码背后的缘由。

查看此 CL 之前的注释也会有所帮助。也许有一个待办事项现在可以删除，有意见建议不要进行此更改，等等。

请注意，注释与类，模块或函数的文档不同，注释应表示一段代码的用途，应如何使用以及使用时的行为。

代码风格

谷歌为主要编程语言和大多数次要编程语言提供了代码风格指南，确保 CL 遵循适当的风格指南。

如果您想改善指南中没有的样式点，请在注释前面加上“Nit：”，以使开发人员知道这是您认为可以改善代码但不是强制性的选择。但请不要只是基于个人偏好来阻止 CL 的提交。

开发人员不应将主要风格更改与其他更改结合在一起。这使得很难看到 CL 中的更改，使合并和回滚更加复杂，并导致其他问题。例如，如果开发人员想要重新格式化整个文件，请他们仅将重新格式化的格式作为一个 CL 发送给评审人员，然后再发送另一个具有功能更改的 CL。

文档

如果 CL 改变了用户构建，测试，与代码交互或释放代码的方式，请检查其是否还更新了相关文档，包括 README，g3doc 页面和其他生成的参考文档。如果 CL 删除或弃用了代码，请考虑是否还应删除该文档。如果文档缺失，要向开发人员索要。

每一行代码

查看每一行代码。有时可以扫描诸如数据文件，生成的代码或大型数据结构之类的东西，但不要扫描人工编写的类，函数或代码块，并假设其中的内容是可以正常运行的。显然，某些代码比其他代码更需要仔细检查 —— 至于是哪些代码完全取决于你，但你至少应该要理解这些代码都在做些什么。

如果代码很复杂或者你难以快速看懂它们，这会使评审速度变慢，那么你应该让开发人员知道这一点，并等待他们解释，然后再尝试评审。Google 聘请了出色的软件工程师，如果他们看不懂代码，其他开发人员也很可能看不懂。因此，要求开发人员进行解释，也可以帮助将来的开发人员理解此代码。

如果您理解代码，但又没有资格进行代码评审，请确保在 CL 上有一位合格的评审人员，特别是在复杂性问题（例如安全性，并发性，可访问性，国际化等）上。

上下文

通常，代码查看工具只会显示需要更改部分的几行代码。但是有时必须查看整个文件，以确保更改确实有意义。例如，你可能会看到仅添加了四行代码，但是当你查看整个文件时，您会看到这四行位于 50 行方法中，这个时候需要将其分解为较小的方法。

需要基于整个系统来考量 CL。此 CL 是在改善系统的代码运行状况，还是使整个系统更加复杂，更不可测等？不要接受会降低系统代码运行状况的 CL。大多数系统会通过许多小的更改而变得复杂，因此，防止新更改中出现很小的复杂性是很重要的。

好的方面

如果在 CL 中看到不错的东西，请告诉开发人员，尤其是当他们以出色的方式回答了你的评论时。代码评审通常只关注错误，但是也应该鼓励和赞赏良好的实践。有时候告诉开发人员正确的做法比告诉他们错误的做法更有价值。

总结

在进行代码评审时，你应确保：

• 代码经过精心设计。
• 该功能对代码的用户来说有用。
• UI 的变更合理。
• 任何并行编程都是安全完成的。
• 代码复杂性不要超过应有的程度。
• 不需要实现可能会在未来出现的需求。
• 代码具有适当的单元测试。
• 精心设计的测试用例。
• 开发人员对所有内容都清晰的命名。
• 清晰而有用的代码注释，要解释“为什么”，而不是“什么”。
• 恰如其分的代码文档化。
• 该代码符合我们的风格指南。

确保检查的每一行代码，查看上下文，确保改善代码运行状况，并称赞开发人员所做的出色工作。

检查 CL

摘要

在知道了代码评审要关注哪些东西之后，如何有效地进行跨文件代码评审呢？

• 更改代码有意义吗？它有一个很好的描述吗？
• 首先看一下变化中最重要的部分，整体设计得如何？
• 以适当的顺序查看其余的 CL。

第一步：全面了解代码变更

查看 CL 说明以及 CL 的一般功能。这种变更有意义吗？如果这个变更是不必要的，请立即做出答复，并说明为什么不应该进行更改。当您拒绝这样的更改时，最好还是向开发人员建议他们应该做些什么。

例如，您可能会说：“看起来您为此做了一些出色的工作，谢谢！但是，实际上我们正打算移除这个系统，因此我们现在不希望对其进行任何新的修改。或许，您可以重构我们的新 BarWidget 类吗？”

请注意，评审人员在拒绝当前的 CL 时要一并提出其他建议，而且还要表现地礼貌。这种礼貌很重要，因为作为开发人员，即使我们不同意，也要表明我们彼此尊重。

如果出现多个 CL 是你不想进行的更改，则应考虑重新设计团队的开发流程或外部贡献者的已发布流程，以便在编写 CL 之前进行更多的沟通。最好在人们完成大量现在必须扔掉或彻底重写的工作之前告诉他们“不”。

第二步：检查 CL 的主要部分

查找属于此 CL 的“主要”部分的文件。通常，逻辑更改数量最多的文件是 CL 的主要部分。首先看这些主要部分。这有助于为 CL 的所有较小部分提供上下文，并通常加快执行代码评审的速度。如果 CL 太大，您无法确定哪些部分是主要的，请询问开发人员您应该首先看什么，或要求他们将 CL 分成多个 CL。

如果您发现 CL 的这一部分存在一些主要的设计问题，要立即回复开发人员，即使您现在没有时间评审 CL 的其余部分。实际上，复查 CL 的其余部分可能会浪费时间，因为如果设计问题足够严重，那么许多其他代码都会变得无关紧要。

为什么要立即回复开发人员有两个主要原因：

• 开发人员通常会发出一个 CL，然后在等待审核时立即基于该 CL 进行新工作。如果您正在评审的 CL 中存在重大设计问题，那么他们也将不得不重新设计其以后的 CL。所以，最好赶在开发人员在有问题的设计上花费不必要的时间之前告诉他们。
• 重大的设计变更比小的变更需要更长的时间。为了在截止日期之前完成工作，并在代码库中保留高质量的代码，开发人员需要尽快开始 CL 的所有重写工作。

第三步：按适当顺序检查其余的 CL

确认 CL 整体上没有大的设计问题后，请尝试找出逻辑顺序来浏览文件，同时还要确保不要错过对任何文件的评审。通常，在浏览了主要文件之后，按照代码评审工具向您展示它们的顺序浏览每个文件是最简单的。有时在阅读主要代码之前先阅读测试也是有帮助的，因为这样您就可以知道更改应该做什么。

代码审查速度

为什么代码审查应该很快？

在谷歌，我们对开发团队的整体交付速度（而不是针对个体开发人员写代码的速度）进行了优化。个体开发速度也很重要，但其重要性比不上整个团队的开发速度。

当代码评审速度很慢时，会发生以下几件事：

• 团队的整体开发速度降低了。如果个体开发人员无法快速地对评审做出响应，可能是因为他们有其他事情要做。但是，如果每个 CL 都要等待一次又一次的评审，那么团队其余成员的新功能和错误修复就会被延迟了几天，几周或几个月。
• 开发人员开始抗议代码评审过程。如果评审人员仅每隔几天回复一次，但每次都要求对 CL 进行重大更改，那么这对于开发人员来说可能是非常令人沮丧且困难的。通常，这表示为对评审人员的“严格”程度的抱怨。如果评审人员能够快速提供反馈（确实可以改善代码运行状况的更改），抱怨就会消失，即使他们要求做出的修改是一样的。实际上，大多数对代码评审过程的抱怨都可以通过加快评审过程来解决。
• 代码的质量可能会受到影响。当评审速度缓慢时，开发人员的压力也会随之增加，因为他们不能提交不甚完美的 CL。缓慢的评审流程还会阻碍代码清理、重构和对现有 CL 做出进一步改进。

代码审查应该有多快？

如果你不是在集中精力完成手头的任务，那就应该在第一时间评审代码。

一个工作日是响应代码检查请求所需的最长时间（即第二天早上的第一件事）。

遵循这些准则意味着典型的 CL 应在一天之内进行多轮评审（如果需要）。

速度与中断

有一种情况，个人速度的考虑胜过团队速度。如果您正在处理诸如编写代码之类的重点任务，请不要打断自己去进行代码检查。研究表明，开发人员在中断之后要花很长时间才能恢复到平稳的开发流程。因此，在编写代码时打断自己，实际上对团队来说，要比让其他开发人员稍等一会儿进行代码评审更为昂贵。

所以，对于这种情况，可以等到你手头工作可以停了再开始代码评审。可以是在完成手头的编码任务之后，午饭后，会议结束后，休息结束后等。

快速响应

我们所说代码评审速度指的是响应时间，而不是 CL 通过整个评审流程并提交所花的时间。整个过程在理想情况下也应该是很快的，但单次评审请求的响应速度比整个过程的响应速度更重要。

即使有时需要很长时间才能完成整个评审过程，在整个评审过程中获得评审人员的快速响应也可以极大地缓解开发人员对“缓慢”代码评审的不满。

如果您忙于在 CL 上进行全面评审时，仍然可以先发送一个快速响应，以使开发人员知道什么时候可以开始评审，或者建议让其他可以更快做出响应的评审人员来评审代码，或者提供一些初步反馈。

重要的是，评审人员应花费足够的时间进行评审，以确保此代码符合标准。但不管怎样，最好响应速度还是要快一些。

跨时区代码评审

在处理跨时区代码评审时，请在他们仍在办公室时尝试与开发人员联系。如果他们已经回家了，那么最好可以确保他们在第二天回到办公室时可以看到代码评审已经完成。

LGTM 带注释

为了加速代码评审速度，在某些情况下，即使审核人员也将未解决的评论留在了 CL 上，评审人员仍应该给予 LGTM/ 批准。在以下情况之一时完成此操作：

• 评审人员确信开发人员将会处理好评审人员给出的建议和意见。
• 其余的改动是次要的，不一定要求开发人员完成。

除非另有明确说明，否则评审人员应使用这些选项中的一个。

当开发人员和评审人员处于不同时区时，带有注释的 LGTM 特别值得考虑，否则开发人员将等待一整天才获得“LGTM，批准”。

大型的 CL

如果有人向您发送了一个很大的代码评审，您不确定何时可以有时间对其进行评审，通常的响应应该是要求开发人员将 CL 拆分为多个相互构建的较小的 CL。而不必一次审查所有巨大的 CL。这样通常是合理的，并且对评审人员很有帮助，即使需要开发人员做些额外的工作。

如果不能将一个 CL 分解为较小的 CL，并且您没有时间快速评审整个 CL，那么至少要对 CL 的总体设计写一些注释，然后将其发回给开发人员进行改进。作为评审人员，您的目标之一应该是是在不影响代码质量的情况下快速对开发人员做出响应，或者让他们能够快速采取进一步行动。

持续代码评审的改进

如果您遵循了这些准则，并且严格执行代码评审，则应该发现整个代码评审过程会随着时间的流逝而越来越快。开发人员知道为了保证代码质量需要做些什么，并从一开始就向你发送非常棒的 CL，这样评审所需的时间就会越来越少。评审人员学会如何快速做出响应，并且不会在评审过程中增加不必要的延迟。但是，不要在代码评审标准或质量上做出让步以提高速度 —— 从长远来看，这不会使任何事情更快地发生。

紧急情况

在某些紧急情况下，CL 必须非常快速地通过 整个审核过程，并且质量准则将得到放宽。但是，请参阅什么是紧急情况？描述哪些情况实际上可以视为紧急情况，哪些不可以。

评审注释

摘要

• 礼貌。
• 说明您的理由。
• 在给出明确的指示与指出问题并让开发人员决定之间做出权衡。
• 鼓励开发人员简化代码或添加代码注释，而不仅仅是让他们解释复杂性。

礼貌

通常，重要的是要礼貌和尊重，同时也要对要查看其代码的开发人员非常了解。一种方法是确保您的注释是针对代码，而是对开发人员。肃然不必总是遵循这种做法，但是在说出可能会令人沮丧或引起争议的内容时，一定要使用它。例如：

不好的说法：“为什么要在这个地方使用线程，这样做显然不会获得任何好处”

好的说法：“并发模型在这里增加了系统的复杂性，而我没有看到任何实际的性能优势。由于没有任何性能上的好处，因此最好将此代码为单线程而不是使用多个线程。”

解释理由

从上面的正确示例中可以看出，这样有助于开发人员理解为什么要给出这些建议。您不一定总是需要在评论注释中包含此信息，但是有时适当的做法可以成为对您的意图的理解，所遵循的最佳实践或您的建议如何改善代码质量进行更多说明。

提供指导

通常，修复 CL 是开发人员的责任，而不是评审人员的责任。您无需执行解决方案的详细设计或为开发人员编写代码。

但是，这并不意味着评审人员不应该给予帮助。通常，您应该在指出问题和提供直接指导之间取得适当的平衡。指出问题并让开发人员做出决定通常可以帮助开发人员学习，并使代码评审更容易。这也可以带来更好的解决方案，因为开发人员比评审人员更接近代码。

但是，有时直接给出指令指令，建议甚至代码会更有帮助。代码评审的主要目的是获得最佳的 CL。第二个目的是提高开发人员的技能，以便他们之后的评审会越来越少。

接受注释

如果您要求开发人员解释一段您不理解的代码，他们通常会去重写代码。有时，在代码中添加注释也是一种适当的响应，只要它不只是解释过于复杂的代码即可。

仅在代码检查工具中编写的说明对将来的代码阅读者没有帮助。只有少数情况可以接受这种做法，例如，你对评审的东西不太熟悉，而开发人员的解释却是很多人所熟知的。

代码评审回推

有时，开发人员会回推代码评审。他们可能会不同意您的建议，或者会抱怨您总体上过于严格。

谁是对的？

当开发人员不同意您的建议时，请先花点时间考虑一下它们是否正确。通常，它们比您更接近代码，因此他们实际上可能对代码的某些方面有更好的了解。他们的论点有意义吗？从代码质量的角度来看，这有意义吗？如果是这样，请让他们知道他们是对的，然后问题就解决了。

但是，开发人员并不总是正确的。在这种情况下，评审人员应进一步解释为什么他们认为自己的建议正确。良好的解释不仅说明了对开发人员的理解，而且还说明了为何要求他们进行更改其他信息。

如果评审人员认为他们的建议可以改善代码质量，并认为评审所带来的代码质量改进值得开发人员做出额外的工作，那么他们就应该坚持。改善代码质量往往是由一系列的小步骤组成的。

有时，在真正提出建议之前，需要花很多时间去解释。请确保始终保持礼貌，并让开发人员知道您了解他们在说什么，您只是不同意。

烦恼的开发人员

评审人员有时认为如果他们坚持要开发人员做出改动，会使开发人员感到沮丧。有时，开发人员确实会感到不高兴，但这通常是短暂的，之后他们会非常感谢您帮助他们提高了代码质量。如果您表现的很有礼貌，那么开发人员根本不会感到不开心，这种担心可能是多余的。烦恼通常与评审人员的注释编写方式有关，而不是与评审人员对代码质量的坚持。

稍后解决

回退的一个常见原因是开发人员想要完成任务（可以理解）。他们不想仅仅为了获得此 CL 而进行另一轮评审。因此，他们说他们将在以后的 CL 中清理某些内容，因此评审人员现在应该 LGTM 此 CL。一些开发人员对此非常好，并会立即编写后续的 CL 来解决此问题。但是，经验表明，开发人员编写原始 CL 的时间越长，他们进行后续修复的可能性就越小。实际上，除非开发人员在提交当前的 CL 之后立刻修复，否则在通过之后通常不会再去做这件事情。这不是因为开发人员不负责任，而是因为他们有很多工作要做，所以修复工作通常会被遗忘。因此，最好是坚持要求开发人员在代码进入代码库并“完成”之前立即修复其 CL。让开发人员“稍后解决”是代码库退化的一种常见方法。

如果 CL 引入了新的复杂性，除非是紧急情况，否则必须在提交之前将其清除。如果 CL 暴露了一些问题，并且现在无法解决，那么开发人员应把错误记录下来，分配给自己，以免丢失。他们还可以选择在引用已提交错误的代码中编写 TODO 注释。

抱怨评审过于严格

如果您以前对代码的评审不是那么严格，但是却突然变得严格起来，那么一些开发人员将会大声抱怨。不过没关系，提高代码评审的速度通常会使这些抱怨消失。

有时，这些抱怨可能需要经过数月的时间才会消失，但是最后开发人员会看到严格的代码评审的价值，因为他们会看到严格的代码评审帮助自己产出优秀代码。有时候，如果发生这种事情，声音最强烈的抗议者甚至会成为您最坚强的支持者。

解决冲突

如果您遵循上述所有内容，但是在代码评审过程中仍然遇到无法解决的冲突，请再次参阅以上内容以获取有助于解决冲突的指导准则。

原文链接：How to do a code review