Google 的工程实践指南 (下)：代码开发者指南

原文由知無涯发表于TesterHome社区，点击原文链接可与作者直接交流。

1.编写良好的 CL 描述

CL 描述是一项公开的记录，其内容包含修改了什么与为什么这么修改。虽然你的 CL 只是在你与审核者之间发生，但它是版本控制历史的一部分，若干年之后，很有可能会有成百上千的人阅读。

以后的开发者可能会根据描述搜到你以前写的 CL 。在没有精确数据的情况下，他可能根据自己的模糊记忆搜索 CL。如果所有的信息都只包含在代码里面，描述中几乎没有相关内容，那么定位到你的 CL 将会花费太多的时间。

第一行

● 简短描述做了什么。
● 完整的句子，祈使句。
● 后面空一行。

CL 描述的第一行应该是一句简短的描述，用以说明 CL做了什么。在第一行后面留一空行。以后有开发者搜索版本控制历史的代码时，这是他们看到的第一行，所以第一行应该提供足够的信息，他们不必阅读代码，也不必阅读整个描述，只需扫一眼便知道 CL 做什么，他们节省时间。

一般而言，CL 描述的第一行是以命令口吻（祈使句）写的一句话。举例说明，我们应该写“ 删除 FizzBuzz RPC，并用写的系统替换它。”，而不是写成 “ 删除了 FizzBuzz RPC，并已经用写的系统替换它。” 当然，第一行写成祈使句就可以了，其他内容不必如此。 (译者注：原文中的反面例子是现在进行时。但在中文中现在进行时与祈使句基本一致，不好翻译。此处改成了现在完成时。)

描述内容要提供充分的信息

描述内容应该提供足够的信息。它可能包含一段关于问题的简短描述，为什么这是最好的解决方案。如果有更好的解决方案，也应该提及。如果有的话，相关的背景信息，如 bug 编号、基准结果和相关的设计文档也应包含在内。

即使是小的 CL，也应该包含这些信息。

糟糕的 CL 描述
“修复 bug”是一个很不恰当的描述。哪个 bug ？你做了哪些事情来修复它？通通都没有。类似糟糕的描述还包括：
● “修复编译。”
● “增加补丁。”
● “把代码从 A 移到 B。”
● “第一阶段。”
● “增加方便的功能。”
● “清除死链。”
以上这些都是我们在真实案例中见过的 CL 描述。作者可能认为他们提供了足够的信息，其实它们不符合 CL 的目的描述。

良好的 CL 描述
这是几个良好描述的样例。
1.功能修改
RPC：移除 RPC 服务器的消息空闲列表的大小限制。
服务器（如 FizzBuzz）有大量的消息，可以从重用中受益。使空闲列表更大，并添加一个goroutine，随着时间的推移缓慢释放空闲列表，以便空闲服务器最终释放所有空闲列表。
前面几句话描述了 CL 做什么的，接下来描述解决了什么问题，为什么这是一个好的解决方案，最后涉及到了一些实现细节。

2.重构
构建一个带 TimeKeeper 的 Task，以便使用它的 TimeStr 和 Now 方法。在 Task 中增加一个 Now 方法，然后删掉 borglet() 方法（这个方法仅仅被 OOMCandidate 使用，它调用了 borglet 的 Now 方法）。这样就替换掉 Borglet的方法，把它委托给 TimeKeeper。

让 Tasks 提供 Now 是减少对 Borglet 的依赖所做的一小步。最终，从 Task 上调用 Now 的方式会替代成直接调用 TimeKeeper，我们会逐步实现。

继续重构 Borglet 层次的长期目标。

第一行描述了 CL 做什么的，以及过去它是怎么改变的。描述的其他部分谈到了具体实现、CL 的上下文，这种方法并不完美，但在朝着完美的方向前进。而且也解释了为什么应该这么改。

3.需要一些上下文的小 CL
为 status.py 创建一个 Python3 的编译。
在原始的编译（Python2）旁创建一个 Python3 的编译，让已经使用过 Python3 编译的用户根据某些规则选择 Python3 还是 Python2，而不是依赖于某个路径。它鼓励新用户尽可能使用 Python3 而不是 Python2，并大大简化了当前正在使用的某些自动编译文件重构工具。

第一句话描述做了什么，其他部分解释为什么要这么修改，并向审核者提供了不少额外的上下文信息。

提交 CL 之前审核描述

在审核过程中，CL 可能会发生重大改变（与最初提交审核的 CL 相比）。在提交 CL 之前有必要再审视一遍 CL 描述，确保描述能够正确地反映 CL 做了什么。

2.小 CL

为什么应该写小 CL？

小 CL 有如下优点：
● 相对让审核者单独拿出30分钟审核大 CL，不如让他花费几个5分钟审核代码。对审核者而言，后者更容易。
● 审核更彻底。发生较大修改时，往往会反复审核、修改，审核者与开发者经常会因为过多的反复而在情绪上受到影响，以致于把精力放在修改上了，却忽略了 CL 中更重要的部分。
● 引入新缺陷的可能性更小。如果修改的内容比较少，自然审核人的效率会更高，开发者与审核者都更容易判断是否引入了新的缺陷。
● 如果被拒绝，浪费的时间更少。如果开发者花费了很大的精力开发了一个大 CL，直到审核的时候才知道整个开发的方向错了，那么之前的所有时间就全浪费了。
● 更容易合并代码。大 CL 在合并代码时会花费很长的时间，在合并时需要花费大量时间，而且在写 CL 期间可能不得不频繁地合并。
● 更易于设计。完善小 CL 的设计和修改要容易得多，多次微小的代码质量提高比一次大的设计改变更容易。
● 减少阻塞审核的可能性。小 CL 通常是功能独立的部分，你可能正在修改很多代码（多个小 CL），在发送一个 CL 审核时，同时可以继续修改其他的代码，并不会因为当前 CL 的审核没有完成而阻塞。
● 更容易回退。一个大 CL 开发的时间比较长，这意味从开发到代码提交这段期间，代码文件的变更会比较多。当回退代码时，情况会变得很复杂，因为所有中间的 CL 很有可能也需要回退。

请注意：审核者有权因为你的 CL 太大而拒绝它。通常，他们会感谢你为代码做出的贡献，但是会要求你把它拆分多个小 CL。一旦写好了代码之后，要把它拆分成小 CL 通常需要花很多时间，当然，你也可能会花费大量时间与审核者争论为什么他应该接受这个大 CL 。与其如此，不如设计之初就保证 CL 尽量小。

如何定义“小”？

一般而言，一个 CL 的大小就应该是独立功能的修改。
这意味着：
● 一个 CL 尽量最小化，它只做一件事。通常它只是一个功能的一部分，而不是整个功能。总体而言，CL 太小或太大都不好，二者取其轻的话，太小稍微好点。可以与审核者一起讨论，找出大多比较合适。
● 审核者需要理解 CL 中包含的一切（除了以后可能要开发的功能之外），包括 CL 代码、描述、已存在的代码（或之前已经审核过的相关 CL ）。
● 在 CL 代码提交之后，无论是针对用户，还是针对开发人员，系统应该仍旧运行良好。
● 如果代码难以理解，通常是因为 CL 还不够小。如果新增一个 API，同时应该同一个 CL 中附上这个 API 的使用方法，便于审核者理解如何使用，也方便以后的开发者理解。同时也可能有效防止提交的 API 无人使用。

没有直观的标准判断 CL “太大”应该符合哪些条件。 100行代码通常是一个合理的大小。1000行代码通常太大了，但也不能一概而论，这取决于审核者的判断。修改文件的个数也影响它的“大小”。在一个文件中修改200行可能没问题，如果这200行代码横跨50个文件，通常而言太大了。

记住，当你开始编写代码时，只有你最了解代码，而审核者通常不了解上下文。在你看起来很是一个合适大小的 CL，审核者可能会很困惑。毫无疑问，在写 CL 时，CL 的大小最好比自认为的还要小。审核者通常不会抱怨你的 CL 太小了。

什么时候可以有大 CL？

当然，也有一些例外情形，允许 CL 比较大：
● 删除一个文件与修改一行没有太大区别，因为它不会花费审核者太多时间。
● 有时候，一个大 CL 可能是由可靠的自动代码重构工具生成的，审核者的工作主要是检查它是否做了它应该做的工作。虽然符合以上提到的注意事项（例如合并和测试），这类 CL 也可能比较大。

以文件来拆分

另外一种拆分大 CL 的方法是：对 CL 中涉及的文件进行分组，这就要求不同独立功能的修改需要相应的审核者。

例如：你提交了一个 CL，这个 CL 修改了协议缓冲区，而且另外一个 CL 用到它。因此我们先提交第一个 CL，再提交第二个 CL，并让两个 CL 同时审核。此时，你应分别向两个 CL 的审核者告知另外一个 CL 的内容，以便他们知道上下文。

以代码和配置文件进行拆分。例如，你提交了2个 CL：其中一个 CL 修改了一段代码，另外一个 CL 调用了这段代码或代码的相关配置；当需要代码回滚时，这也比较容易，因为配置或调用文件有时候推送到产品比代码修改相对容易。

单独重构

在修改功能或修复缺陷的 CL 中，不建议把重构也加进来，而是建议把它放到单独的 CL 中。例如，修改类名或把某个类移到其他包内是一个 CL，修复这个类中的某个缺陷是一个 CL，不要把它们合并到一个 CL 中。把它们拆分出来更有利于审核者理解代码的变化。

有些代码清理工作，如修改某个类中的一个变量的名称，可以把它包含在一个功能修改或缺陷修复的 CL 中。那标准是什么呢？这取决开发者与审核者的判断，这种重构是否大到让审核工作变得很困难。

把测试代码包含到对应功能的 CL 中

避免单独提交测试代码。测试代码用以验证代码功能，应该把它与代码提交到相同的 CL 中，虽然它会增大 CL 的代码行数。

然而，独立的测试修改可以放到单独的 CL 中，这与单独重构中的观点比较类似。它包含如下内容：
● 为过去提交的已存在代码创建新的测试代码。
● 重构测试代码（例如，引入 helper 函数）。
● 引入测试框架代码（如，集成测试）。

不要破坏编译

如果同时在审核的有多个 CL，并且这些 CL 之间存在依赖关系，你需要找到一种方式，确保依次提交 CL 时，保证整个系统仍旧运行良好。否则，可能在提交某个 CL 之后，让系统编译错误。此时，你的同事在更新代码后，不得不花时间查看你的 CL 历史并回退代码以确保本地编译没有问题（如果你之后的 CL 提交出了问题，可能会花费更多时间）。

无法将其变小

在某些情形下，好像你没法然 CL 变得更小，这种情况很少发生。如果开发者经常写小 CL，那么他往往都能找到一种把 CL 拆得更小的方法。

如果在写代码之前就估计这个 CL 比较大，此时应该考虑是否先提交一个代码重构的 CL，让已有的代码实现更清晰。或者，与团队其他成员讨论下，看是否有人能帮你指出，怎样在提交小 CL 的前提下实现当前功能。

如果以上所有方法都试过，还是不可行（当然，这种情况比较罕见），那就先与所有的审核者沟通一下，告知他们你将会提交一个大 CL，让他们先有心理准备。出现这种情况时，审核过程往往会比较长，同事需要写大量的测试用例。需要警惕，不要引入新的 bug。

3.如何处理审核者的评论

在发出 CL 之后，审核者一般会给出反馈（评论），让你修改代码。下面我们就来详细描述如何处理这些评论。

不要情绪化

代码审核的目标是保证提交到的代码库中代码的质量，进而保证产品的质量。当审核者提出一些批判性的评论时，开发者应该告诉自己，对方在尝试帮助你，保证代码库的质量，帮助 Google，而不是针对你的人身攻击或个人能力的怀疑。

有时候，审核者感到很沮丧，并在评论中表达了这种心情。其实，这不是一种正确的方式，但作为开发者，你应该有足够的心理准备来面对这种情况。问一下自己，“我能从审核者的评论中读出哪些建设性的意见？”假想他们就是以这种建设性的语气对你说的，然后按照这种建议去做。

不要在代码审核中带着情绪回复评论。在审核代码过程中，违反专业礼仪是件很严重的事情，但我们永远没法确保别人不违反专业礼仪。我们可以保证自己不要违反它。如果你很生气或恼火，以致于无法友好地回复，那就离开电脑一会儿，或者先换一件事情做直到心态平静下来，再礼貌地回复。

一般情况下，如果审核者没有以建设性地口吻提供反馈，反馈的方式不够礼貌，可以私下与他沟通。如果不方便与他私下沟通，也不方便通过是视频电话远程沟通，可以给他单独发一封邮件。在邮件中，以友好的态度向他解释，你很难接受这种反馈方式，期待他能换一种沟通方式。如果他仍旧以一种非建设性的方式回复你，或没有达到预期效果，那就升级到他的主管吧。

修复代码

如果审核者说，他对你的代码中有些内容不理解，你的第一反应是澄清代码本身。如果代码无法澄清，加一段注释用以解释为什么这段代码这样写。如果这段注释放到代码里毫无意义，那就应该放到代码评审工作的评论中作为反馈的解释。

如果审核者无法理解某段代码，很有可能其他的代码阅读者也不懂。在代码审核工具中回复它对未来的代码阅读者没有任何好处。这种情况应该尝试清理代码，或增加一些必要的注释，以帮助他们阅读。

先思考自己是否有改进的空间

写一个 CL 会耗费大量精力。在提交一个 CL 审核时，开发者会往往会产生几乎快要完成的幻觉，自认为无需进一步修改。当审核者回复需要修改某些代码时，开发者容易条件反射地认为反馈不正确，审核者没必要阻碍自己的开发，他应该让这个 CL 审核通过。然而，无论你有多确定这点，最好还是先退一步，仔细考虑审核者是否在反馈中提供了有价值的内容，可以帮助提高代码库的质量。你的第一个问题应该永远都是，“审核者说得对吗？”
如果无法回答这个问题，很有可能审核者需要进一步澄清评论。

如果你已经思考过，并确认你是对的，那就在回复中解释为什么你的方法比较好（相对已有的代码、用户）。通常，审核者也会提供建议，他们希望你比较一下哪个更好。你可能知道一些关于用户、代码库或 CL的内容，而这些正是审核者不了解的，那么就把这些写出来，提供更多的上下文。通常，你在技术上可以与审核者达成一致。

冲突解决

在解决冲突的第一步永远都是应该先尝试与审核者达成共识。如果无法达成共识，可以参考审核代码标准，当面临这种情形时，它为我们提供了一些准则。

4.紧急情况

有时候会有紧急的 CL。紧急情况发生时，必须尽快完成审核流程并提交。

哪些是紧急情况？

紧急 CL 应该是一个小修改：一个重要的发布版本需要包含某个功能（无法回滚），修复产品中严重影响用户的缺陷，处理紧迫的法律问题，关闭一个重要的安全漏洞，等等。
处于紧急情况时，我们应该关心整个代码审核流程的速度，而不仅仅是响应的速度。更准确地说，在这种情况下，审核者应该把审核速度与代码的正确性（代码是否解决了紧急问题？）放在首位。并且，当紧急情况发生时，它的审核优先级必须高于其他所有的代码审核。
当紧急情况处理完毕之后，应该回过头来再继续做一次更全面的审核。

哪些不是紧急情况？

需要明确的是，如下情形不是紧急情况：
● 希望本周完成，而不是下周（除非有一些无法避免的硬期限，如合作伙伴之间的契约）。
● 开发者为这个功能已经开发了很长一段时间，他们希望尽快提交代码。
● 所有的审核者都不在相同的时区，他们现在是半夜或下班时间。
● 现在是周五快下班的时间，如果开发者能在周末离开公司之前提交代码那就太棒了。
● 经理说这个审核必须完成，CL 在今天必须提交因今天是截止日期(软期限，而非硬期限)。
● 回滚一个造成测试失败或编译错误的 CL。等等。

什么是硬期限？

所谓硬期限（hard deadline），就是错过了就会有灾难性的事情发生。例如：
● 合同规定，你必须在某个特定日期之前提交 CL。
● 如果产品没有在某个特定日期之前发布，很有可能会影响销量，甚至在产品市场导致失败。
● 有些硬件制造商每年只会发布一次产品。如果在截止日期之前没有提交代码给他们，而这些代码又非常重要，很有可能会造成灾难性的后果。

推迟一周发布不是灾难性的。错过某次重要的会议可能是灾难性的，也可能不是。

大多数截止日期都是软期限（soft deadline），并非是不可改变的。这些软期限期望在某个时间节点得到需要的功能。它们很重要，但不应该为了达到目标而破坏代码的健康状况。
如果发布周期较长（好几个月），则很有可能会牺牲代码质量，以期在下一个周期之前实现某个功能。如果这种模式反复出现，那就为项目筑起了难以承受的技术债，这是项目开发中常见的问题。如果开发者经常在临近开发周期结束的时候提交代码，那么团队“必定”只能做表面上的代码审核。此时，团队应该修改开发流程，大型的功能修改应该在周期的早期进行，以确保有足够的时间做好代码审核。