1 简介
Google具有许多涵盖所有语言和所有项目的通用工程实践，这些文档代表了我们随着时间的推移积累的各种最佳实践的集体经验。开源项目或其他组织可能会从此知识中受益，因此我们致力于在可能的情况下将其公开提供。
1.1 术语
这些文档中有一些使用Google内部术语，我们在此向外部读者阐明这些术语：
- CL：代表“变更列表”（
ChangeList），表示一个已包含到版本控制中或正在进行代码评审的更改。其他组织通常将其称为“更改”或“补丁”。 - LGTM：代码评审者在批准CL时所说的“在我看来这个CL很好”的缩写，即
look good to me的缩写。 - Nit: 全称
nitpick，意思是鸡蛋里挑骨头。
1.2 指南说明
在Google，我们通过代码评审来保证代码和产品的质量。代码评审是一个过程，在此过程中，代码段作者以外的人来评审这段代码。本文档是Google代码评审流程和政策的规范描述。
Google的代码评审指南，实际上包含两组独立的文档：
- 如何进行代码评审？
- CL开发者指南
2 如何进行代码评审？
2.1 引言
2.1.1 代码评审到底评审什么
代码评审应关注的点：
- 设计：代码是否经过精心设计并适合你的系统？
- 功能：代码是否符合开发者的预期意图？代码对用户的表现是否良好?
- 复杂性：可以简化代码吗？将来当其他开发人员遇到该代码时，他们是否能够轻松理解和使用该代码？
- 测试：是否有正确且设计良好的自动化测试？
- 命名：开发人员是否为变量，类，方法等选择了清晰的名称？
- 注释：这些注释是否清晰有用？
- 风格：代码是否遵循我们的风格指南?
- 文档：开发者是否也更新了相关的文档？
2.1.2 选择最佳的评审者
通常，你希望找到能够在合理的时间内对你的代码做出回复的最佳评审者。
最好的评审者应该是能够为你写的代码片段提供最彻底、最正确的评审的人。通常，代码的拥有者是OWNERS中的人，但也可能不是。这意味着有时需要不同的人来查看CL的不同部分。
如果找到了理想的评审者但他们却没有时间，至少要将更改的内容抄送给他们。
2.1.3 自我评审
如果你与某人一起对一段代码进行编程，并且他有资格对代码进行良好的代码评审，则该代码将被视为已评审。
你还可以进行自我代码评审（即自己既扮演开发者，又扮演评审者）。评审者会提出问题，而CL的开发者只有在被提问时才能回答。
2.2 代码评审标准
代码评审的主要目的是确保Google代码库的整体代码运行状况随着时间的推移而不断改善。所有代码评审工具和过程都是为此目的而生。
为了实现这一点，必须做出一些折衷。
首先，开发人员必须能够在其任务上取得进展。如果你从未向代码库提交过改进，那么代码库将永远不会得到改进。另外，如果评审人员让任何更改都很难进行，那么开发人员将没有动力做出一些改进。
另一方面，评审者有责任保证每个CL的质量，以使得代码库的整体代码运行状况不会随着时间的流逝而减少。有种情况比较棘手：因为随着时间的推移，通常会由于每次CL小幅的下降而导致代码库整体的退化，尤其是当团队处于明显的时间限制下并且他们认为必须采取捷径才能实现其目标时。
此外，评审者对他们正在评审的代码拥有所有权和责任。他们需要确保代码库的一致性、可维护性、以及2.1.1节“代码评审到底评审什么”中提到的所有其他事项。
因此，我们期望将以下规则作为代码评审中的标准：
通常，即使CL并不完美，但可以肯定它能改善正在使用的系统的整体代码运行状况，则评审者就应该通过它。
这是所有代码评审指南中的高级原则。
当然，这是有局限性的。例如，如果CL添加了评审者不希望在其系统中使用的功能，那么即使代码设计得当，评审者也可以拒绝通过它。
这里一个关键点是：没有“完美”的代码，只有更好的代码。评审人不应要求开发者在批准前将CL的每一小块都做优化。相反，与他们所建议的更改的重要性相比，评审者应该平衡取得进步的需要。评审者追求的不是完美，而是持续改进。总体而言，提高系统的可维护性，可读性和可理解性的CL不能因为它不是“完美的”而被延迟几天或几周通过。
评审者应该随时对可以优化的代码发表评论，但是如果不是很重要，可以在其前面加上“ Nit：”之类的字眼，以使作者知道这只是他们可以选择忽略的优化点。
2.2.1 指导
代码评审具有重要的功能，可以教给开发者有关语言、框架或通用软件设计原理方面的新知识。留下有助于开发者学习新知识的评论总是很好的。共享知识是随着时间的推移而改善系统代码运行状况的一部分。请记住，如果你的评论纯粹是教育性的，但对于满足本文档中描述的标准不是至关重要的，请在其前缀前面加上“ Nit：”，或者对代码提交者表明并非强制要求在本次CL中一定要对其解决。
2.2.2 原则
- 技术事实和数据要大于意见和个人偏好。
- 在代码样式方面，样式指南是绝对权威的。不在样式指南中的任何纯样式点（如空格等）都是个人喜好问题。样式应与代码中的样式保持一致。如果没有以前的样式，请接受开发者的样式。
- 软件设计方面几乎从来都不是纯粹的样式问题，也不是个人喜好问题。 它们基于基本原则，应权衡这些原则，而不仅仅是个人意见。 有时会有一些有效的其他选择，如果开发者可以证明（通过数据或基于可靠的工程原理）几种方法同样有效，那么评审者应接受开发者的偏爱。 否则，最终的选择需要由软件设计的标准原理来决定。
- 如果没有其他规则适用，则评审者可以要求开发者与当前代码库中的内容保持一致，只要这不会使系统的总体代码运行状况恶化。
2.2.3 解决冲突
在代码评审中发生任何冲突时，第一步应始终是基于本指南促使开发者和评审者达成共识。
当达成共识变得特别困难时，在评审者和开发者之间进行面对面的会议或VC可能会有所帮助，而不仅仅是尝试通过代码评审注释来解决冲突。（但如果这样做了，请确保将讨论的结果记录在CL上的注释中，以备将来阅读。）
如果那不能解决问题，则最常见的解决方法是升级。升级的途径通常是进行更广泛的团队讨论，其中包括TL的权衡，要求代码维护者的决定或要求Eng经理提供帮助。不要因为开发者和评审者不能达成一致意见而让CL一直放在那里。
2.3 代码评审到底评审什么？
2.3.1 设计
评审中最重要的内容是CL的总体设计。CL中各种代码的交互是否有意义？此更改是属于你的代码库还是lib包？它与系统的其余部分是否集成良好？现在是添加此功能的好时机吗？
2.3.2 功能性
此CL是否达到开发者的预期目的？开发者打算为该代码的用户带来什么好处？ 这里的“用户”通常既是最终用户（当此代码更改时影响到的人）又是开发人员（将来他们将不得不“使用”此代码的人）。
通常，我们希望开发者能够对CL进行良好的测试，以确保它们在进行代码评审时能够正常工作。但是，作为评审者，你仍然应该考虑边缘情况，寻找并发性问题，尝试像用户一样思考，并确保没有仅仅通过阅读代码就能看到的错误。
你可以根据需要验证CL，对于评审者而言，检查CL的行为最重要的时间点是对用户产生影响时（例如UI更改）。当你仅仅阅读代码时，很难理解某些更改将如何影响用户。对于这样的更改，如果过于麻烦而无法自己尝试，则可以让开发人员为你提供功能演示。
在代码评审时，另一个需要特别重点考虑功能性的时间点是：CL中是否正在进行某种并行编程，从理论上讲它可能会导致死锁或竞争条件。仅通过运行代码很难检测到这类问题，通常需要有人（开发人员和评审者）仔细考虑它们，以确保不会引入问题。 （请注意，这也是在可能出现竞争条件或死锁的情况下不使用并发模型的一个很好的理由，这会使进行代码评审或理解代码非常复杂。）
2.3.3 复杂性
CL是否本不需要如此复杂？ 对CL的每个维度都要进行检查——个别行是否太复杂？ 功能太复杂了吗？ 类太复杂了吗？ “太复杂”通常意味着“代码阅读者无法快速理解”。 这也可能意味着“开发者在尝试调用或修改此代码时更可能引入bug”。
一种特殊类型的复杂性是过度设计，即开发人员使代码变得比需要的通用，或者增加了系统目前不需要的功能。评审者应特别警惕过度设计。鼓励开发人员解决他们现在需要解决的已知问题，而不是解决开发者认为将来可能需要解决的问题。
2.3.4 测试
根据更改需求进行单元测试、集成测试或端到端测试。通常，应在与生产代码相同的CL中添加测试，除非CL在处理紧急情况。
确保CL中的测试正确、合理且有用。测试代码不会测试他们自己，我们也很少为测试代码编写测试脚本，所以必须要确保测试是有效的。
代码损坏时，测试会执行失败吗？ 如果代码基于它来更改，将会产生误报吗？每个测试都会做出简单而有用的断言吗？ 测试是否在不同的测试方法之间适当地分开？
请记住，测试代码也是必须维护的。 不要仅仅因为它们不是主逻辑的一部分而接受测试代码的复杂性设计。
2.3.5 命名
开发人员是否为代码中所有的变量、类、方法等选择了一个好名字？ 一个好名字需要足够长以充分传达它的含义或作用，但又不能太长而令人难以阅读。
2.3.6 注释
开发人员是否用可理解的英语写下了清晰的注释？所有注释实际上都是必要的吗？ 通常，解释了为什么存在某些代码的注释是非常有用的，而不应该解释某些代码在做什么时。如果代码不够清晰，且无法解释自身，则应使代码更简单。有一些例外情况（例如，正则表达式和复杂算法通常会在注释中解释它们的作用，从而大大受益），但大多数注释是针对代码本身无法包含的信息，例如决策背后的原因。
查看此CL之前的注释可能对你也会有所帮助。比如，也许有一个TODO现在可以删除了、有意见或建议不要进行此更改等等。
请注意：注释与类、模块或函数的文档不同，注释应表示一段代码的用途，包括它应如何使用以及使用时的会发生什么。
2.3.7 代码样式
Google提供了所有主要语言甚至大多数次要语言的样式指南。 确保CL遵循适当的样式指南。
如果你想改善样式指南中没有的样式点，请在注释前面加上“ Nit：”，以使开发人员知道这是你认为可以改善代码但不是强制性的要求。不要阻挠仅根据个人风格偏好提交的CL。
CL的开发者不应将主要样式更改与其他更改混在一起。这使得很难看到CL中的更改，合并和回滚也会变得更加复杂，并导致其他问题。例如，如果开发者想要重新格式化整个文件，请他们仅将重新格式化的格式作为一个CL发送给你，然后再发送另一个具有功能更改的CL。
2.3.8 文档
如果CL改变了用户构建、测试、与代码交互或释放代码的方式，请检查其是否还更新了相关限的文档，包括README文件、页面和任何生成的参考文档。如果CL删除或弃用了代码，请考虑是否还应删除该文档。如果缺少文档，请提给开发者。
2.3.9 每一行代码
查看已分配给你检查的每一行代码。有时可以扫描诸如数据文件、生成的代码或大型数据结构之类的东西，但不要扫描人工编写的类、函数或代码块，并假设其中的内容是没问题的。显然，某些代码比其他代码更需要仔细检查——这是你必须要做出的判断，但你至少应确保你了解所有代码在做什么。
如果对你来说某段代码太难阅读了，这会使评审速度变慢，那么你应该让开发人员知道这一点，并让他们解释一下，然后再尝试评审。在Google，我们聘请的都是出色的软件工程师，你也是其中之一。如果你看不懂这段代码，其他开发人员也很可能看不懂。 因此，当你要求开发者进行解释时，你还可以帮助将来的开发人员理解此代码。
如果你了解代码，但又认为没有资格进行某部分的评审，请确保在CL上有一位合格的评审员，特别是在复杂性问题上（例如安全性、并发性、可访问性、国际化等）。
2.3.10 上下文
在广泛的上下文下查看CL通常会很有帮助。 通常，代码查看工具只会向你显示围绕要更改的部分的几行代码。有时，你必须查看整个文件，以确保更改确实有意义。例如，你可能会看到仅添加了四行，但是当你查看整个文件时，你会看到这四行分散到一个50行的方法中，所以开发者需要将其分解为较小的方法。
在整个系统的上下文中考虑CL也很有用。此CL是在改善系统的代码运行状况，还是使整个系统更加复杂，测试更少等？不要接受会降低系统代码运行状况的CL。大多数系统会通过许多小的更改而变得复杂，因此，重要的是要防止新更改中出现很小的复杂性。
2.3.11 好的事情
如果你在CL中看到不错的东西，请告诉开发人员，尤其是当他们以出色的方式回答了你的评论之一时。代码评审通常只关注错误，但是他们也应该鼓励和赞赏良好实践。在指导方面，有时候告诉开发人员正确的做法比告诉他们哪里做错了更有价值。
2.3.12 总结
在进行代码评审时，你应确保：
- 代码经过精心设计。
- 功能对代码的用户来说要友好。
- UI的任何更改都是有效的，并且看起来不错。
- 任何并行编程都是安全完成的。
- 代码实现没有比需要的复杂。
- 开发人员没有实现他们将来可能需要的东西，但不知道他们现在需要什么。
- 代码具有适当的单元测试。
- 测试代码经过精心设计。
- 开发者对所有内容都使用了清晰的名称。
- 注释是清晰而有用的，并且大多数解释了原因而不是什么。
- 代码已正确文档化（通常在g3doc中）。
- 代码符合我们的样式指南。
确保评审要求检查的每一行代码、查看上下文、确保改善代码的运行状况、并称赞开者所做的出色工作。
2.4 浏览评审中的CL
2.4.1 摘要
既然你已经知道”评审代码什么到底在评审什么"了，那么管理分散在多个文件中的代码评审的最有效方法是什么呢？
- 这个更改有意义吗？它有一个很好的描述吗？
- 看一下变化中最重要的部分，它整体设计得好吗？
- 以适当的顺序查看其余的CL。
2.4.2 第一步：全面了解变化
通常，通过查看CL的描述以及CL的功能来了解更改。但这种更改有意义吗？如果发现不应该进行此更改，请立即做出回复，并说明为什么不应该进行更改。当你拒绝这样的更改时，最好向开发者建议一下他们应该做些什么。
例如，你可能会说：“看起来你在其中做了一些出色的工作，谢谢！但是，我们实际上正朝着删除你在此处修改的FooWidget系统的方向发展，因此我们不想现在对其做任何新的修改。相反，重构我们的新BarWidget类如何？”
请注意，评审者不仅拒绝了当前的CL并提出其他建议，而且还礼貌地做到了。这种礼貌很重要，因为即使我们不同意，作为开发者我们也想表明我们彼此尊重。
如果收到多个你不想更改的CL，则应考虑重新设计团队的开发流程或已发布的外部贡献者的流程，以便在编写CL之前进行更多的沟通。最好在人们完成大量现在必须扔掉或彻底重写的工作之前告诉他们“不”。
2.4.3 第二步：检查CL的主要部分
查找属于此CL的“主要”部分的文件。通常，如果一个文件的逻辑更改数量最多，那么它就是CL的主要部分。首先看这些主要部分。这有助于为CL的所有较小部分提供上下文，并通常加快执行代码评审的速度。如果CL太大，你无法确定哪些部分是主要零件，请询问开发者你应该首先看什么，或要求他们将CL分成多个CL。
如果你发现CL的这一部分存在一些主要的设计问题，则即使你现在没有时间评审CL的其余部分，也应立即发送这些评论。实际上，评审CL的其余部分可能会浪费时间，因为如果设计问题比较严重，那么正在评审的许多其他代码都将消失并且无论如何都不会变得很重要。
立即发出这些主要的设计评论有两个主要原因：
- 开发人员通常会在发出一个CL之后立即基于该CL进行新工作。如果你正在评审的CL中存在重大设计问题，那么他们也将不得不重新设计其以后的CL。在他们在有问题的设计之上进行过多额外工作之前，你就需要告诉他们。
- 重大的设计变更比小的变更需要更长的时间。开发者几乎都有截止日期；为了保证在截止日期之前完成工作，并在代码库中保留高质量的代码，开发者需要尽快开始CL的所有重大重做。
2.4.4 第三步：以适当的顺序浏览其余的CL
一旦你确认CL再整体上没有大的设计问题之后，请尝试找出逻辑顺序来浏览文件，同时还要确保不要错过对任何文件的评审。通常，在浏览了主要文件之后，按照代码评审工具向你展示的顺序浏览每个文件是最简单的。有时在阅读主要代码之前先阅读测试代码也是有帮助的，因为这样你就可以知道这些更改的意图是什么了。
2.5 代码评审的速度
2.5.1 为什么代码评审要快速完成？
在Google，我们优化了开发团队可以一起生产产品的速度，而不是优化了单个开发人员可以编写代码的速度。个人发展的速度固然很重要，但并没有整个团队的速度更重要。
当代码评审缓慢时，会发生几件事：
-
整个团队的速度下降。对评审没有快速回应的个人来说，他可以完成其他工作。但由于每个CL都在等待评审，因此团队其余成员的新功能和bug修复会被延迟几天、几周或几个月。 -
开发人员开始抗议代码评审过程。如果评审者仅每隔几天回答一次，但每次都要求对CL进行重大更改，那么这对于开发者来说可能会令人沮丧且困难。 通常情况下，这表现为对评审者的过于“严格”的抱怨。如果评审者要求进行实质性的更改（确实可以改善代码运行状况的更改），且每次开发者进行更新时都做出快速响应，则抱怨往往会消失。实际上，大多数对代码评审过程的抱怨都可以通过加快速度来解决。 -
代码的健康状况可能会受到影响。当审查变慢时，允许开发者提交不尽可能好的CLs的压力就增大了。缓慢的审核也会阻止代码清理、重构和对现有CL的进行进一步改进。
2.5.2 怎样快速的进行代码评审呢？
如果你没有在做一件需要集中精神的任务，你应该在CL出现后不久后就做一下代码评审。
一个工作日是响应代码评审请求所需的最长时间（即第二天早上的第一件事）。
遵循这些准则意味着一个典型的CL应在一天之内进行多轮评审（如果需要）。
2.5.3 速度 vs 中断
有时个人的速度比团队的速度更为重要。如果你正在集中精神做某件任务，比如编写代码，请不要打断自己去做代码评审。 研究表明，开发人员在中断之后要花很长时间才能恢复到平稳的开发流程。 因此，在编写代码时打断自己实际上对团队来说要比让其他开发者稍等一会儿的代价更为昂贵。
相反，在你的工作中等待一个断点，然后再响应一个评审请求。这可能是当你当前的编码任务完成后、午饭后、从会议返回、从微厨房返回等等。
2.5.4 快速响应
当我们谈论代码评审的速度时，它指的是我们的响应时间，而不是CL通过整个评审并提交所花的时间。整个过程在理想情况下也应该是快速的，但是个人的响应要比整个过程迅速完成更为重要。
尽管有时需要很长时间才能完成整个评审过程，但在整个评审过程中获得评审者的快速响应也可以极大地减轻开发人员对“缓慢”代码评审的沮丧感。
如果你太忙，无法在CL进入时对其进行完整的评审，你仍然可以发送一个快速的响应，让开发人员知道你何时可以访问它，或建议其他可能能够更快地响应的评审者来评审，或者提供一些初步的宽泛的评论。（注意：这些都不意味着你应该中断编码来发送这样的响应，而是在工作的合理断点时发送响应。）
重要的是，评审者应花足够的时间进行评审，以确保其“ LGTM”表示“此代码符合我们的标准”。 但理想情况下，个人响应仍然应该很快。
2.5.5 跨时区的评审
在处理时区差异时，尽量在作者还在办公室时联系他。如果他们已经回家了，那么在他们第二天回到办公室之前，试着确保你的评审已经完成。
2.5.6 带评论的LGTM
为了加快代码评审的速度，在某些情况下，评审者应给予LGTM/批准，即使他们也在CL上留下未解决的评论。这是在以下情况下才会出现的：
- 评审者确信开发者将适当地处理评审者留下的所有评论。
- 所作的更改很小，开发者不必做。
除非另有明确说明，否则评审者应指定他们打算使用这些选项中的哪一个。
当开发者和评审者位于不同时区时，带有注释的LGTM特别值得考虑，否则开发者将等待一整天才获得“ LGTM，批准”。
2.5.7 巨大的CLs
如果有人向你发送了一个巨大的代码评审，你不确定何时可以有时间对其进行评审，通常的响应应该是要求开发人员将CL拆分为多个较小的CL。而不必一次评审所有巨大的CL。这通常是可能的，并且对审阅者很有帮助，即使需要开发人员进行一些额外工作。
如果不能将一个CL分解为较小的CL，并且你没有时间快速评审整个CL，那么至少要对CL的总体设计写一些评论，然后将其发给开发者进行改进。 作为评审者，你的目标之一应该是始终消除开发人员的障碍，或在不牺牲代码的运行状况的情况下，使他们能够迅速采取某种进一步的措施。
2.5.8 随着时间的推移，代码评审的改进
如果你遵循这些准则，并且严格执行代码评审规范，则应该发现整个代码评审过程会随着时间的流逝而越来越快。 开发人员将了解健康代码的要求，并从一开始就向你发送出色的CL，所需的评审时间越来越少。评审者学会快速做出响应，并且不会在评审过程中增加不必要的延迟。但是，不要为了提高速度而在代码评审标准或质量上做出让步——从长远来看，这实际上不会使任何事情更快地发生。
2.5.9 紧急情况
在某些紧急情况下，CL必须非常快速地通过整个审核过程，并且质量准则将得到放宽。但是，请参阅什么是紧急情况？ 描述哪些情况实际上可以视为紧急情况，哪些不可以。
2.6 如何编写代码评审评论
2.6.1 摘要
- 友善
- 说明你的理由
- 在给出明确的指导与指出问题、并让开发人员决定之间保持平衡。
- 鼓励开发人员简化代码或添加代码注释，而不是仅仅向你解释其复杂性。
2.6.2 礼貌
通常，有礼貌和尊重是重要的，同时也要对正在查看其代码的开发者非常有帮助。 一种方法是确保始终对代码进行评论，而不对开发者进行评论。不必总是遵循这种做法，但是在说出可能令人不快或有争议的内容时，一定要使用它。 例如：
坏：“当显然不能从并发中获得好处时，为什么在这里使用线程？”
好：“并发模型在这里增加了系统的复杂性，而我没有看到任何实际的性能优势。因为没有性能优势，所以最好使该代码为单线程而不是使用多个线程。”
2.6.3 解释为什么
关于上面的“好”示例，你会注意到一件事，那就是它帮助开发人员理解你发表评论的原因是什么。你并不总是需要在评论中包含这些信息，但有时需要对你的意图、遵循的最佳实践或如何改善代码的运行状况的建议给出更多的解释。
2.6.4 提供指导
一般来说，修复CL是开发人员的责任，而不是评审人员的责任。你不需要为开发人员进行解决方案的详细设计或编写代码。
但是，这并不意味着审阅者是没有帮助的。通常，你应该在指出问题和提供直接指导之间取得适当的平衡。指出问题并让开发人员做出决定通常可以帮助开发人员学习，并使代码评审更容易。这也可以带来更好的解决方案，因为开发者比审阅者更接近代码。
然而，有时直接的指导、建议，甚至代码更有用。代码评审的主要目标是获得尽可能好的CL。第二个目标是提高开发人员的技能，以便他们需要越来越少的评审时间。
2.6.5 接受说明
如果你让开发人员解释一段你不理解的代码，通常会导致他们更清楚地重写代码。偶尔，在代码中添加注释也是一种适当的响应，只要它只是仅仅解释过于复杂的代码。
仅在代码检查工具中编写的说明对将来的代码阅读者没有帮助。它们仅在某些情况下是可以接受的，例如，当你查看不十分熟悉的领域并且开发者解释了普通代码读者已经知道的内容时。
2.7 在代码评审中处理回退
有时，开发人员会退回进行代码评审。他们要么是因为对你的建议不同意，要么是因为抱怨你总体上过于严格。
2.7.1 到底谁是对的？
当开发人员不同意你的建议时，请先花点时间考虑一下它们是否正确。通常，它们比你更接近代码，因此他们实际上可能对代码的某些方面有更好的了解。他们的论点有意义吗？从代码健康角度来看，这有意义吗？如果是这样，请让他们知道他们是对的，然后让问题解决。
但是，开发者并不总是对的。在这种情况下，评审人应进一步解释为什么他们认为自己的建议是正确的。良好的解释不仅说明了对开发人员回复的理解，而且还说明了有关为何要求进行更改的其他信息。
特别是，当评审者认为他们的建议将改善代码的健康状况时，如果他们认为由此产生的代码质量改进证明所要求的额外工作是合理的，那么他们应该继续提倡进行更改。改善代码的健康状况往往只需要一小步。
有时候，在一个建议真正被采纳之前，它需要几轮的解释。只要确保始终保持礼貌，让开发人员知道你听到了他们在说什么，只是你不同意而已。
2.7.2 令人烦恼的开发者
评审人有时认为，如果评审人坚持要改进，开发者会感到沮丧。有时，开发人员确实会感到不高兴，但这通常是短暂的，之后他们会非常感谢你帮助他们提高了代码质量。通常，如果你的评论彬彬有礼，那么开发人员实际上根本不会感到烦恼，而这些担心只会出现在评审者的脑海中而已。烦恼通常与评论的编写方式有关，而不是与评审人对代码质量的坚持有关。
2.7.3 稍后再清理
回退的一个常见原因是开发人员（可以理解）想要完成任务。他们不想仅仅为了通过此CL而进行另一轮审核。因此，他们说他们将在以后的CL中清理某些内容，因此你现在应该LGTM此CL。一些开发人员对此非常友好，并会立即编写后续的CL来解决此问题。然而，经验表明，随着开发人员编写原始CL之后时间的推移，这种清理发生的可能性越小。实际上，通常除非开发人员在当前CL之后立即进行清理，否则它永远不会发生。这不是因为开发人员不负责任，而是因为他们有很多工作要做，并且清理工作在其他工作中丢失或被遗忘。因此，通常最好是坚持要求开发人员在代码进入代码库并“完成”之前立即清理其CL。让人们“以后再清理”是代码库退化的一种常见方法。
如果CL引入新的复杂性，它必须在提交之前进行清理，除非是紧急情况。如果CL暴露了相关的问题，而这些问题现在无法解决，那么开发人员应该提交一个清理错误并将其分配给自己，这样它就不会丢失。他们还可以选择在代码中编写一个TODO注释来引用已归档的bug。
2.7.4 对严格评审的抱怨
如果你以前对代码的评审松懈，而转而对代码进行严格的评审，那么一些开发人员将大声抱怨。提高代码评审的速度通常会使这些抱怨消失。
有时，这些抱怨可能要花费数月的时间才能消失，但是最终，开发人员往往会看到严格代码评审的价值，因为他们会看到自己写出的出色代码。有时，最大声的抗议者甚至会成为你最有力的支持者，一旦有什么事情发生，使他们真正看到你通过严格要求所增加的价值。
2.7.5 解决冲突
如果你遵循上述所有内容，但是仍然遇到无法解决的开发人员之间的冲突，请参阅"代码评审标准"以获取有助于解决冲突的准则和原则。
3 CL开发者指南
3.1 编写良好的CL描述
CL描述是一个公开的记录，它记录了正在进行的更改以及更改的原因。它将成为版本控制历史中永久的一部分，并且可能会被数百人阅读，而不是仅仅是你的评审者。
未来的开发者将会根据你的CL的描述来搜索它。将来可能有人会因为只有相关的模糊的记忆，却没有具体的细节而查找你的CL。如果所有的重要信息都在代码中而不是描述中，那么他们很难找到你的CL。
3.1.1 第一行
- 对正在做的事情做一个简短的总结。
- 完整的句子，命令式的写法
- 紧接着的空行
CL描述的第一行应该是对CL正在做的具体工作的简短总结，后面跟一个空行。这是大多数未来的代码搜索者在浏览一段代码的版本控制历史时会看到的，因此第一行应该有丰富的信息量，他们不必阅读你的CL或它的整个描述就可以大致了解你的CL实际做了什么。
按照传统，CL描述的第一行是一个完整的句子，写成好像是一个命令（命令式句子）。例如，说"Delete the FizzBuzz RPC and replace it with the new system."，而不是"Deleting the FizzBuzz RPC and replacing it with the new system."。另外，你不必将其余说明写成命令式语句。
3.1.2 正文内容要丰富
其余的描述应该是翔实的。它可能包括对正在解决的问题的简短描述，以及为什么这是最好的方法。如果这个方法有任何缺点，应该提出来。如果相关，请包括背景信息，如错误号、基准测试结果和设计文档的链接。
即使是很小的CLs也需要注意其中的一些实现细节。
3.1.3 糟糕的CL描述
“Fix bug”是对CL的不足描述。什么bug?为了修复这个bug，你做了什么？其他类似的错误描述包括：
- "Fix build."
- "Add patch."
- "Moving code from A to B."
- "Phase 1."
- "Add convenience functions."
- "kill weird URLs."
上面这些CL描述在现实场景中是真实存在的。这些开发者可能认为他们正在提供有用的信息，但它们并没有达到CL描述的目的。
3.1.4 良好的CL描述
以下是一些很好的CL描述示例。
3.1.4.1 功能变更
例如：
rpc: remove size limit on RPC server message freelist.
Servers like FizzBuzz have very large messages and would benefit from reuse. Make the freelist larger, and add a goroutine that frees the freelist entries slowly over time, so that idle servers eventually release all freelist entries.
第一行描述了CL的实际作用。描述的其余部分讨论了要解决的问题、为什么这是一个好的解决方案，以及有关实现的更多信息。
3.1.4.2 重构
例如：
Construct a Task with a TimeKeeper to use its TimeStr and Now methods.
Add a Now method to Task, so the borglet() getter method can be removed (which was only used by OOMCandidate to call borglet's Now method). This replaces the methods on Borglet that delegate to a TimeKeeper.
Allowing Tasks to supply Now is a step toward eliminating the dependency on Borglet. Eventually, collaborators that depend on getting Now from the Task should be changed to use a TimeKeeper directly, but this has been an accommodation to refactoring in small steps.
Continuing the long-range goal of refactoring the Borglet Hierarchy.
第一行描述了CL的作用，以及它与过去的变化。描述的其余部分讨论了具体的实现、CL的上下文、解决方案并不理想以及未来可能的方向。同时也解释了为什么会做这种修改。
3.1.4.3 需要某些上下文的小型CL
例如：
Create a Python3 build rule for status.py.
This allows consumers who are already using this as in Python3 to depend on a rule that is next to the original status build rule instead of somewhere in their own tree. It encourages new consumers to use Python3 if they can, instead of Python2, and significantly simplifies some automated build file refactoring tools being worked on currently.
第一句话描述了实际正在做的事情。描述的其余部分解释了为什么要进行更改，并为评审者提供了大量的上下文。
3.1.5 提交CL之前请先检查描述
CLs在评审期间可能发生重大变化。在提交CL之前，有必要检查一下CL的描述，以确保该描述仍然反映CL的功能。
3.2 小型的CLs
3.2.1 为什么要编写小型CL？
小型，简单CL的意义是：
-
评审的更快。比起留出30分钟的时间来评审一个大的CLs，评审者更容易找到5分钟的空闲来评审小的CLs。 -
评审的更加彻底。进行较大的更改后，评审者和作者往往会因大量来来回回的回复而感到沮丧，有时某些重要的评论还会被遗漏掉。 -
引入bug的可能性更小。由于你所做的更改较少，因此你和你的评审者更容易有效地推断出CL的影响，并查看是否会引入某些bug。 -
如果被拒绝，浪费的工作也会更少。如果你编写了一个巨大的CL，然后你的审阅者说总体方向是错误的，那么你就浪费了很多工作。 -
更容易合并。在大型CL上工作需要很长时间，因此合并时你将会遇到很多的冲突，你不得不进行频繁合并。 -
更易于设计。完善小变更的设计和代码运行状况要比完善大变更的所有细节容易得多。 -
更不容易卡在评审上发送整体更改中的一部分可以让你在等待当前CL审核时继续进行编码。 更易于回滚。
请注意，审阅者有权直接拒绝你的CL，唯一的原因是更改太大。通常他们会感谢你的贡献，但要求你以某种方式把它变成一系列较小的变化。在你已经写了一个变更之后，分割它可能需要很多工作，或者你需要花很多时间来和评审人来争论为什么应该接受你的大变更。所以，一开始就写一些小的CLs是比较容易的。
3.2.2 为什么要提交小型的CL？
通常，一个CL的正确大小应该是一项独立的更改。这意味着：
- CL进行的最小更改只解决一个问题。这通常只是一个功能的一部分，而不是一个完整的功能。一般来说，编写太小的CLs比编写太大的CLs更好。与你的评审人一起找出一个可接受的大小。
- 评审人需要了解的关于CL的一切（除了未来的功能）都在CL、CL的描述、现有的代码库、或他们已经评审的CL中。
- CL被合并之后，系统将继续为其用户和开发人员良好运行。
- CL不能太小以至于难以理解。如果你添加了一个新的API，则应在同一CL中包括该API的用法，以便评审人可以更好地了解该API的使用方式。这也可以防止检入未使用的API。
对于“太大”，没有硬性规定。对于CL而言，100行通常是一个合理的大小，而1000行通常太大，但这取决于你的评审者的判断。一个更改所覆盖的文件数也会影响它的“大小”。一个文件中200行的更改可能是可以的，但是覆盖50个文件通常会太大。
请记住，尽管从你开始编写代码的那一刻起，你就与代码密切相关，但是评审人通常没有上下文。对你来说，一个大小尚可接受的CL可能会让你的评审人难以承受。如果有疑问，可以编写比你认为需要编写的更小的CLs。评审人很少会因CLs太小而抱怨。
3.2.3 什么时候提交大型CL也是可以的？
在某些情况下，较大的更改没有那么糟糕：
- 通常，你可以将整个文件的删除视为一行的更改，因为它不会花费很长时间来评审。
- 有时候，一个你完全信任的自动重构工具会生成一个很大的CL，评审者的工作就是检查并确认他们确实需要更改。这些CLs可以更大，尽管有上面的一些警告(如合并和测试)但它仍然适用。
3.2.3.1 按文件拆分
另一种分割CL的方法是对文件进行分组，这些文件需要不同的评审人员评审，而且又是独立的更改。
例如：你发送一个CL来修改协议缓冲区，而发送另一个CL来更改使用该协议的代码。你必须在协议的CL之前提交修改的CL，但它们可以同时被评审。如果执行此操作，则可能要告知两组评审者有关你编写的另一个CL的信息，以便他们拥有进行更改的上下文。
另一个示例：你发送一个CL进行代码更改，而另一个CL则发送使用该代码的配置或实验；如果需要，这也更容易回滚，因为有时将配置/实验文件推送到生产环境中比更改代码更快。
3.2.4 单独的重构
通常，最好在一个独立的CL中进行重构，而不是进行特性更改或bug修复。例如，移动和重命名一个类应该与修复该类中的错误属于不同的CL。当每个CL独立时，评审员更容易理解它们所引入的更改。
不过，可以在特性更改或bug修复CL中包含修复局部变量名之类的小清理。由开发人员和评审者来决定怎么样的重构算是庞大的，以至于大到如果被包含在当前的CL中，评审将变得更加困难。
3.2.5 将相关的测试代码保存在同一CL中
避免将测试代码拆分为单独的CL。即使这样做会增加代码行数，验证代码修改的测试也应该进入相同的CL。
然而，类似于重构准则，可以将独立的测试修改放入单独的CL中。包括：
- 使用新测试验证先前存在的提交代码。
- 重构测试代码（例如，引入辅助函数）。
- 引入更大的测试框架代码（例如集成测试）。
3.2.6 不要破坏构建
如果你有几个相互依赖的CLs，那么你需要找到一种方法来确保在提交每个CL之后整个系统都能继续工作。否则，你可能会在CL提交之间将所有其他开发人员的构建中断几分钟(如果在以后的CL提交中出现意外的问题，则可能需要更长的时间)。
3.2.7 无法将CL拆分成小的？
有时你会遇到一些情况，似乎你的CL必须很大，但这往往都是假象。练习编写小型CLs的开发者几乎总能找到将功能分解为一系列小更改的方法。
在编写大型CL之前，请考虑在这之前加上只进行重构的CL是否可以为以后简洁的实现做好铺垫。与你的队友交谈，看看是否有人对如何在小型CLs中实现该功能有想法。
如果所有这些选项都失败了（这应该是极为罕见的），那么请事先获得评审人员的同意来评审一个大型的CL，这样他们就会被预先提醒接下来会发生什么。在这种情况下，估计要经历很长时间的评审过程，注意不要引入bug，并在编写测试时格外小心谨慎。
3.3 如何处理评审者的评论？
当你将CL发送出去进行审核时，你的评审人很可能会对你的CL做出一些评论。这里是有关如何处理审阅者评论的一些有用信息。
3.3.1 不要往心里去
评审的目的是保证我们代码库和产品的质量。当评审者对你的代码提出批评时，请尝试将其视对你、代码库、Google的帮助，而不是对你或你的能力的人身攻击。
有时，评审人会感到抓狂，并在评论中表达这种抓狂。对于评审者来说，这不是一个好习惯，但是作为开发者，你应该为此做好准备。问问自己，“评审人试图与我交流的建设性内容是什么？”然后像他们实际所说的那样操作。
切勿对代码评审的评论以愤怒的方式回复。这是对职业礼节的严重违反，它将永远存在于代码评审工具中。如果你因太生气或烦恼而无法友好地回答，请离开计算机一段时间，或进行其他操作，直到你感到足够镇静可以礼貌地回答为止。
通常，如果评审人没有以建设性和礼貌的方式提供反馈，请亲自向他们解释。如果你无法当面或视频通话与他们交谈，请向他们发送私人电子邮件。用一种友好的方式向他们解释你不喜欢的事情以及你希望他们做些什么。如果他们也以非建设性的方式对此私密讨论做出回应，或者没有起到预期的作用，请酌情上报给你的经理。
3.3.2 修正代码
如果评审者说他们不理解代码中的某些内容，你的第一个反应应该是通过代码本身来解释。如果代码不能自我解释，请为代码添加注释，解释代码存在的原因。只有当评论看起来毫无意义时，你才可以在代码评审工具中给予回复。
如果评审者不理解你的某些代码，则其他将来的代码读者也可能不理解。在代码检查工具中编写解释不会对将来的代码阅读者有所帮助，但是阐明你的代码或添加代码注释却会对他们有所帮助。
3.3.3 为自己考虑
编写CL需要大量的工作。通常情况下，你会很满足于自己终于发出了一份CL，觉得自己已经完成了任务，并且确信不需要再做其他工作了。因此，当评审人对一些可以改进的地方进行了评论时，很容易让人本能地认为这些评论是错误的、评审者正在不必要地阻碍你、或者他们应该让你提交CL。然而，不管你现在有多确定，花点时间回顾一下，考虑一下评审人是否提供了有价值的反馈，这将有助于代码库和谷歌。你对自己的第一个问题应该是，“评审人是正确的吗?”
如果你无法回答该问题，则评审者可能需要解释他们的评论。
如果你已经考虑过了，但仍然认为你是对的，那么你可以解释一下为什么你的做事方法对代码库、用户或谷歌更好。通常，评论者实际上是在提供建议，他们希望你自己思考什么是最好的。实际上，你可能知道一些评审者并不知道的内容，比如有关用户的、代码库的或CL的。所以把更多有关上下文的信息也写进去吧。通常，你和评审者可以基于一些技术事实来达成某种共识。
3.3.4 解决冲突
解决冲突的第一步应该始终是设法与你的评审者达成共识。如果无法达成共识，请参阅代码评审标准，其中提供了这种情况下应遵循的原则。
网友评论