代码审查标准

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

为了实现这一点，必须权衡一系列折衷。

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

另一方面，审阅者有责任确保每个CL的质量都使得其代码库的整体代码运行状况不会随着时间的流逝而减少。这可能很棘手，因为随着时间的推移，代码库通常会由于代码运行状况的小幅下降而退化，尤其是当团队处于明显的时间限制下并且他们认为必须采取捷径才能实现其目标时。

此外，审阅者对他们正在审阅的代码拥有所有权和责任。他们希望确保代码库保持一致，可维护，以及“代码审查中的查找内容”中提到的所有其他事项 。

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

通常，即使CL并不完美，一旦处于肯定可以改善正在使用的系统的整体代码运行状况的状态下，审阅者就应该批准它。

这是所有代码审查指南中的高级原则。

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

这里的关键点是，没有“完美”的代码，只有更好的代码。审稿人不应要求作者在批准前抛光CL的每一小块。相反，与他们所建议的更改的重要性相比，审阅者应该平衡取得进步的需要。审稿人要追求的不是持续追求完美，而是 持续改进。总体而言，可以提高系统的可维护性，可读性和可理解性的CL不应延迟几天或几周，因为它不是“完美的”。

审阅者应该随时发表评论，表示可能会更好，但是如果不是很重要，请在其前面加上“ Nit：”之类的字样，以使作者知道这只是他们可以选择忽略的亮点。

注意：本文档中没有任何理由证明检查CL肯定会 恶化系统的整体代码运行状况。您唯一要做的是在紧急情况下。

指导

代码审查具有重要的功能，可以教给开发人员有关语言，框架或通用软件设计原理的新知识。留下有助于开发人员学习新知识的评论总是很好的。共享知识是随着时间的推移改善系统代码运行状况的一部分。请记住，如果您的评论纯粹是教育性的，但对满足本文档中描述的标准不是至关重要的，请在其前面加上“ Nit：”，否则，表明并非强制要求作者在此CL中对其进行解决。

原则

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

解决冲突

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

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

如果那不能解决问题，则最常见的解决方法是升级。升级的途径通常是进行更广泛的团队讨论，其中包括TL的权衡，要求代码维护者的决定或要求Eng经理提供帮助。不要让CL坐下来，因为作者和审稿人无法达成协议。

在代码审查中寻找什么

注意：在考虑这些要点时，始终确保考虑到 《代码审查标准》。

设计

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

功能性

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

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

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

在代码审查期间考虑功能性的另一时间特别重要的是，CL中是否正在进行某种并行编程，从理论上讲可能导致死锁或竞争条件。仅通过运行代码很难检测到这类问题，通常需要有人（开发人员和审阅者）仔细考虑它们，以确保不会引入问题。（请注意，这也是在可能出现竞争条件或死锁的情况下不使用并发模型的一个很好的理由，这会使进行代码审查或理解代码非常复杂。）

复杂

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

一种特殊类型的复杂性是过度设计，即开发人员使代码变得比需要的通用，或者增加了系统目前不需要的功能。评论者应特别警惕过度设计。鼓励开发人员解决他们现在需要解决的已知问题，而不是解决开发人员推测 将来可能需要解决的问题。未来的问题一到达就应该解决，您可以在物理宇宙中看到它的实际形状和要求。

测验

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

确保CL中的测试正确，合理且有用。测试不会自我测试，我们很少为测试编写测试-人员必须确保测试有效。

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

请记住，测试也是必须维护的代码。不要仅仅因为它们不是主要二进制文件的一部分而接受测试中的复杂性。

命名

开发人员是否为所有事情都选择了好名字？一个好名字足够长，可以充分传达物品的含义或作用，而又不会太长而难以阅读。

评论

开发人员是否用可理解的英语写下清晰的评论？所有评论实际上都是必要的吗？通常，当注释解释了为什么存在某些代码，而不应该解释某些代码在做什么时，它们很有用。如果代码不够清晰，无法解释自身，则应使代码更简单。有一些例外情况（例如，正则表达式和复杂算法通常会从注释中解释它们的作用而受益匪浅），但大多数注释是针对代码本身可能无法包含的信息，例如决策背后的原因。

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

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

样式

我们风格指南在谷歌为我们所有的主要语言，甚至对于大多数小语种的。确保CL遵循适当的样式指南。

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

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

文献资料

如果CL改变了用户构建，测试，与代码交互或释放代码的方式，请检查其是否还更新了相关文档，包括自述文件，g3doc页面和任何生成的参考文档。如果CL删除或弃用了代码，请考虑是否还应删除该文档。如果缺少文档，请提出要求。

每条线

查看已分配给您检查的每一行代码。有时可以扫描诸如数据文件，生成的代码或大型数据结构之类的东西，但不要扫描人工编写的类，函数或代码块，并假设其中的内容是可以的。显然，某些代码比其他代码更需要仔细检查-这是您必须做出的判断调用-但您至少应确保您了解所有代码在做什么。

如果对您来说太难阅读代码了，这会使审查速度变慢，那么您应该让开发人员知道这一点，并等待他们澄清，然后再尝试审查。在Google，我们聘请了出色的软件工程师，您就是其中之一。如果您听不懂代码，其他开发人员也很可能不会。因此，当您要求开发人员进行澄清时，您还可以帮助将来的开发人员理解此代码。

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

语境

在广泛的背景下查看CL通常会很有帮助。通常，代码查看工具只会向您显示围绕要更改的部分的几行代码。有时，您必须查看整个文件，以确保更改确实有意义。例如，您可能会看到仅添加了四行，但是当您查看整个文件时，您会看到这四行位于50行方法中，现在确实需要分解为较小的方法。

在整个系统的上下文中考虑CL也很有用。此CL是在改善系统的代码运行状况，还是使整个系统更加复杂，测试更少等？不要接受会降低系统代码运行状况的CL。大多数系统会通过许多小的更改而变得复杂，因此，重要的是要防止新更改中出现很小的复杂性。

好东西

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

摘要

在进行代码审查时，您应确保：

• 代码经过精心设计。
• 该功能对代码的用户来说很好。
• UI的任何更改都是明智的，并且看起来不错。
• 任何并行编程都是安全完成的。
• 代码没有比需要的复杂。
• 开发人员没有实现他们将来可能需要的东西，但不知道他们现在需要什么。
• 代码具有适当的单元测试。
• 测试经过精心设计。
• 开发人员对所有内容都使用了清晰的名称。
• 注释清晰而有用，并且大多数解释了原因而不是原因。
• 代码已正确记录（通常在g3doc中）。
• 该代码符合我们的样式指南。

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

浏览评论中的CL

摘要

现在您知道要查找的内容了，管理散布在多个文件中的审阅的最有效方法是什么？

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

第一步：全面了解变化

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

例如，您可能会说：“看起来您为此做了一些出色的工作，谢谢！但是，实际上，我们正朝着删除您在此处修改的FooWidget系统的方向发展，因此我们现在不希望对其进行任何新的修改。相反，您可以重构我们的新BarWidget类吗？”

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

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

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

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

如果您发现CL的这一部分存在一些主要的设计问题，则即使您现在没有时间审查CL的其余部分，也应立即发送这些评论。实际上，复查CL的其余部分可能会浪费时间，因为如果设计问题足够严重，那么正在复查的许多其他代码都将消失并且无论如何都不会变得很重要。

立即发出这些主要设计评论非常重要有两个主要原因：

• 开发人员通常会邮寄一个CL，然后在等待审核时立即基于该CL进行新工作。如果您正在审查的CL中存在重大设计问题，那么他们也将不得不重新设计其以后的CL。您希望在他们在有问题的设计之上进行过多额外工作之前就抓住他们。
• 重大的设计变更比小的变更需要更长的时间。开发人员几乎都有截止日期；为了在截止日期之前完成工作，并在代码库中保留高质量的代码，开发人员需要尽快开始CL的所有重大重做。

第三步：按适当顺序浏览其余的CL

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

代码审查速度

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

在Google，我们优化了一组开发人员可以一起生产产品的速度，而不是优化了单个开发人员可以编写代码的速度。个人发展的速度很重要，但并不像整个团队的速度那么重要。

当代码审查缓慢时，会发生几件事：

• 整个团队的速度下降。是的，对评论没有快速回应的个人可以完成其他工作。但是，由于每个CL等待审查和重新审查，因此团队其余成员的新功能和错误修复被延迟了几天，几周或几个月。
• 开发人员开始抗议代码审查过程。如果审阅者仅每隔几天回答一次，但每次都要求对CL进行重大更改，那么这对于开发人员来说可能会令人沮丧且困难。通常，这表示为对审阅者的“严格”程度的抱怨。如果审阅者要求进行相同的实质性更改（确实可以改善代码运行状况的更改），但是每次开发人员进行更新时都做出快速响应，则抱怨会消失。实际上，大多数对代码审查过程的抱怨都可以通过加快过程来解决。
• 代码的健康状况可能会受到影响。当审核缓慢时，允许开发人员提交的CL可能不尽如人意。缓慢的审核也会阻止代码清理，重构和对现有CL的进一步改进。

代码审查应该有多快？

如果您不在一项重点任务的中间，则应在代码完成后不久进行代码审查。

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

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

速度与中断

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

而是，在您的工作中遇到一个断点，然后再响应审阅请求。这可能是您当前的编码任务完成时，午餐后，从会议返回，从微型厨房回来等时。

快速反应

当我们谈论代码审查的速度时，它是我们所关注的响应时间，而不是CL通过整个审查并提交所花的时间。整个过程在理想情况下也应该是快速的，但是对于个人的响应来说，比整个过程迅速发生更为重要。

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

如果您忙于在CL上进行全面审核时，仍然可以发送快速响应，以使开发人员知道何时可以获取它，建议其他可能会更快响应的审阅者，或者 提供一些初步的广泛评论。（注意：这都不意味着您即使发送这样的响应也不应中断编码-在工作中的合理断点处发送响应。）

重要的是，审阅者应花费足够的时间进行审阅，以确保他们的“ LGTM”表示“此代码符合我们的标准。” 但是，理想情况下，个人答复仍应是快速的。

跨时区评论

在处理时区差异时，请在他们仍在办公室时尝试与作者联系。如果他们已经回家了，请尝试确保您的审核完成，然后他们第二天才回到办公室。

LGTM带评论

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

• 审阅者有信心开发者将适当地解决所有审阅者的其余评论。
• 剩下的变化是轻微的，不具备由开发商来完成。

除非另有明确说明，否则审阅者应指定他们打算使用这些选项中的哪一个。

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

大型CL

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

如果不能将一个CL 分解为较小的CL，并且您没有时间快速审查整个事情，那么至少要对CL的总体设计写一些评论，然后将其发回给开发人员进行改进。作为审阅者，您的目标之一应该是始终解除开发人员的封锁或使他们能够迅速采取某种进一步的措施，而又不牺牲代码的运行状况。

随着时间的推移，代码审查的改进

如果您遵循这些准则，并且严格执行代码审查，则应该发现整个代码审查过程会随着时间的流逝而越来越快。开发人员将了解健康代码的要求，并从一开始就向您发送出色的CL，所需的审查时间越来越少。审阅者学会快速做出响应，并且不会在审阅过程中增加不必要的延迟。但是，不要在代码审查标准或质量上做出让步，以达到想象中的速度改进 -从长远来看，这实际上不会使任何事情更快地发生。

紧急情况

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

如何编写代码注释

摘要

• 善待。
• 说明您的推理。
• 在给出明确的指示与指出问题并让开发人员决定之间保持平衡。
• 鼓励开发人员简化代码或添加代码注释，而不仅仅是向您解释复杂性。

礼貌

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

坏：“为什么显然没有从并发中获得好处，为什么在这里使用线程？”

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

解释为什么

从上面的“好”示例中，您会注意到的一件事是，它可以帮助开发人员理解为什么要发表评论。您不一定总是需要在评论注释中包含此信息，但是有时适当的做法是对您的意图，所遵循的最佳实践或您的建议如何改善代码运行状况进行更多说明。

提供指导

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

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

但是，有时直接的指令，建议甚至代码会更有帮助。代码审查的主要目标是获得最佳的CL。第二个目标是提高开发人员的技能，以便他们随着时间的流逝需要越来越少的审查。

接受说明

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

仅在代码检查工具中编写的说明对将来的代码阅读者没有帮助。它们仅在某些情况下是可以接受的，例如，当您查看不十分熟悉的区域并且开发人员解释了普通代码读者已经知道的内容时。

处理代码审查中的推回

有时，开发人员会推迟进行代码审查。他们要么会不同意您的建议，要么会抱怨您总体上过于严格。

谁是对的？

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

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

尤其是，当审阅者认为他们的建议将改善代码的健康状况时，如果他们认为所导致的代码质量的改善证明所要求的其他工作是合理的，则他们应继续倡导该更改。 改善代码运行状况的步骤往往很短。

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

烦人的开发人员

审稿人有时认为，如果审稿人坚持要改进，开发人员会感到沮丧。有时，开发人员确实会感到不高兴，但这通常是简短的，后来他们非常感谢您帮助他们提高了代码质量。通常，如果您对您的评论彬彬有礼，那么开发人员实际上根本不会感到烦恼，而担心只是在审阅者的脑海中。烦恼通常与注释的编写方式有关 ，而不是与审阅者对代码质量的坚持有关。

稍后清理

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

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

有关严格性的一般投诉

如果您以前对代码的审查松懈，而转而对代码进行严格的审查，那么一些开发人员将大声抱怨。提高代码审查的 速度通常会使这些抱怨消失。

有时，这些抱怨可能要花费数月的时间才能消失，但是最终，开发人员往往会看到严格的代码审查的价值，因为他们会看到自己帮助生成的出色代码。有时候，一旦发生某种事情，使声音最强烈的抗议者甚至成为您最坚强的支持者，这会使他们真正地通过严格地看待您所增加的价值。

解决冲突

如果您遵循上述所有内容，但是仍然遇到无法解决的开发人员之间的冲突，请参阅 《标准代码审查》以获取有助于解决冲突的准则和原则。