代码开发者指南

赢海

本节页面的内容为开发人员进行代码审查的最佳实践。这些指南可帮助您更快地完成审核并获得更高质量的结果。您不必全部阅读它们，但它们适用于每个 Google 开发人员，并且许阅读全文通常会很有帮助。

• 写好 CL 描述
• 小型 CL
• 如何处理审查者的评论
  另请参阅代码审查者指南，它为代码审阅者提供了详细的指导。

写好 CL 描述

CL 描述是进行了哪些更改以及为何更改的公开记录。CL 将作为版本控制系统中的永久记录，可能会在长时期内被除审查者之外的数百人阅读。

开发者将来会根据描述搜索您的 CL。有人可能会仅凭有关联性的微弱印象，但没有更多具体细节的情况下，来查找你的改动。如果所有重要信息都在代码而不是描述中，那么会让他们更加难以找到你的 CL 。

首行

• 正在做什么的简短摘要。
• 完整的句子，使用祈使句。
• 后面跟一个空行。
  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.“ 但是，您不必把其余的描述写成祈使句。

Body 是信息丰富的

其余描述应该是提供信息的。可能包括对正在解决的问题的简要描述，以及为什么这是最好的方法。如果方法有任何缺点，应该提到它们。如果相关，请包括背景信息，例如错误编号，基准测试结果以及设计文档的链接。

即使是小型 CL 也需要注意细节。在 CL 描述中提供上下文以供参照。

糟糕的 CL 描述

“Fix bug ”是一个不充分的 CL 描述。什么 bug？你做了什么修复？其他类似的不良描述包括：

“Fix build.”
“Add patch.”
“Moving code from A to B.”
“Phase 1.”
“Add convenience functions.”
“kill weird URLs.”

其中一些是真正的 CL 描述。他们的作者可能认为自己提供了有用的信息，却没有达到 CL 描述的目的。

好的 CL 描述

以下是一些很好的描述示例。

功能更新

rpc：删除 RPC 服务器消息 freelist 上的大小限制。
像 FIzzBuzz 这样的服务器有非常大的消息，并且可以从重用中受益。增大 freelist，添加一个 goroutine，缓慢释放 freelist 条目，以便空闲服务器最终释放所有 freelist 条目。

前几个词描述了CL实际上做了什么。其余的描述讨论了正在解决的问题，为什么这是一个很好的解决方案，以及有关具体实现的更多信息。

重构

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 的背景，解决方案并不理想，以及未来的可能方向。它还解释了为什么正在进行此更改。

需要上下文的 小 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.

第一句话描述实际做了什么。其余的描述解释了为什么正在进行更改并为审查者提供了大量背景信息。

在提交 CL 前审查描述

CL 在审查期间可能会发生重大变更。在提交 CL 之前检查 CL 描述是必要的，以确保描述仍然反映了 CL 的作用。

小型 CL

为什么提交小型 CL?

小且简单的 CL 是指：

• 审查更快。审查者更容易抽多次五分钟时间来审查小型 CL，而不是留出 30 分钟来审查一个大型 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 的使用方式。这也可以防止签入未使用的 API。

关于多大算“太大”没有严格的规则。对于 CL 来说，100 行通常是合理的大小，1000 行通常太大，但这取决于您的审查者的判断。变更中包含的文件数也会影响其“大小”。一个文件中的 200 行变更可能没问题，但是分布在 50 个文件中通常会太大。

请记住，尽管从开始编写代码开始就您就已经密切参与了代码，但审查者通常不清楚背景信息。对您来说，看起来像是一个可接受的大小的 CL 对您的审查者来说可能是压倒性的。如有疑问，请编写比您认为需要编写的要小的 CL。审查者很少抱怨收到过小的 CL 提交。

什么时候大 CL 是可以的？

在某些情况下，大变更也是可以接受的：

• 您通常可以将整个文件的删除视为一行变更，因为审核人员不需要很长时间审核。
• 有时一个大的 CL 是由您完全信任的自动重构工具生成的，而审查者的工作只是检查并确定想要这样的变更。但这些 CL 可以更大，尽管上面的一些警告（例如合并和测试）仍然适用。

按文件拆分
拆分 CL 的另一种方法是对文件进行分组，这些文件需要不同的审查者，否则就是自包含的变更。

例如：您发送一个 CL 以修改协议缓冲区，另一个 CL 发送变更使用该原型的代码。您必须在代码 CL 之前提交 proto CL，但它们都可以同时进行审查。如果这样做，您可能希望通知两组审查者审查您编写的其他 CL，以便他们对您的变更具有更充足的上下文。

另一个例子：你发送一个 CL 用于代码更改，另一个用于使用该代码的配置或实验；如果需要，这也更容易回滚，因为配置/实验文件有时会比代码变更更快地推向生产。

分离出重构

通常最好在功能变更或错误修复的单独 CL 中进行重构。例如，移动和重命名类应该与修复该类中的错误的 CL 不同。审查者更容易理解每个 CL 在单独时引入的更改。

但是，修复本地变量名称等小清理可以包含在功能变更或错误修复 CL 中。如果重构大到包含在您当前的 CL 中，会使审查更加困难的话，需要开发者和审查者一起判断是否将其拆开。

将相关的测试代码保存在同一个 CL 中

避免将测试代码拆分为单独的 CL。验证代码修改的测试应该进入相同的 CL，即使它增加了代码行数。

但是，独立的测试修改可以首先进入单独的 CL，类似于重构指南。包括：

• 使用新测试验证预先存在的已提交代码。
• 重构测试代码（例如引入辅助函数）。
• 引入更大的测试框架代码（例如集成测试）。

不要破坏构建

如果您有几个相互依赖的 CL，您需要找到一种方法来确保在每次提交 CL 后整个系统能够继续运作。否则可能会在您的 CL 提交的几分钟内打破所有开发人员的构建（如果您之后的 CL 提交意外出错，时间可能会甚至更长）。

如果不能让它足够小

有时你会遇到看起来您的 CL 必须如此庞大，但这通常很少是正确的。习惯于编写小型 CL 的提交者几乎总能找到将功能分解为一系列小变更的方法。

在编写大型 CL 之前，请考虑在重构 CL 之前是否可以为更清晰的实现铺平道路。与你的同伴聊聊，看看是否有人想过如何在小型 CL 中实现这些功能。

如果以上的努力都失败了（这应该是非常罕见的），那么请在事先征得审查者的同意后提交大型 CL，以便他们收到有关即将发生的事情的警告。在这种情况下，做好完成审查过程需要很长一段时间的准备，对不引入错误保持警惕，并且在编写测试时要更下功夫。

如何处理审查者的评论

当您发送 CL 进行审查时，您的审查者可能会对您的 CL 发表一些评论。以下是处理审查者评论的一些有用信息。

不是针对您

审查的目标是保持代码库和产品的质量。当审查者对您的代码提出批评时，请将其视为在帮助您、代码库和 Google，而不是对您或您的能力的个人攻击。

有时，审查者会感到沮丧并在评论中表达他们的挫折感。对于审查者来说，这不是一个好习惯，但作为开发人员，您应该为此做好准备。问问自己，“审查者试图与我沟通的建设性意见是什么？”然后像他们实际说的那样操作。

永远不要愤怒地回应代码审查评论。这严重违反了专业礼仪且将永远存在于代码审查工具中。如果您太生气或恼火而无法好好的回应，那么请离开电脑一段时间，或者做一些别的事情，直到您感到平静，可以礼貌地回答。

一般来说，如果审查者没有以建设性和礼貌的方式提供反馈，请亲自向他们解释。如果您无法亲自或通过视频通话与他们交谈，请向他们发送私人电子邮件。以友善的方式向他们解释您不喜欢的东西以及您希望他们以怎样不同的方式来做些什么。如果他们也以非建设性的方式回复此私人讨论，或者没有预期的效果，那么请酌情上报给您的经理。

修复代码

如果审查者说他们不了解您的代码中的某些内容，那么您的第一反应应该是澄清代码本身。 如果无法澄清代码，请添加代码注释，以解释代码存在的原因。 只有在想增加的注释看起来毫无意义时，您才能在代码审查工具中进行回复与解释。

如果审查者不理解您的某些代码，那么代码的未来读者可能也不会理解。在代码审查工具中回复对未来的代码读者没有帮助，但澄清代码或添加代码注释确可以实实在在得帮助他们。

自我反思

编写 CL 可能需要做很多工作。在终于发送一个 CL 用于审查后，我们通常会感到满足的，认为它已经完成，并且非常确定不需要进一步的工作。这通常是令人满意的。因此，当审查者回复对可以改进的事情的评论时，很容易本能地认为评论是错误的，审查者正在不必要地阻止您，或者他们应该让您提交 CL。但是，无论您目前多么确定，请花一点时间退一步，考虑审查者是否提供有助于对代码库和对 Google 的有价值的反馈。您首先应该想到的应该是，“审查者是否正确？”

如果您无法回答这个问题，那么审查者可能需要澄清他们的意见。

如果您已经考虑过并且仍然认为自己是正确的，请随时回答一下为什么您的方法对代码库、用户和/或 Google 更好。通常，审查者实际上是在提供建议，他们希望您自己思考什么是最好的。您可能实际上对审阅者不知道的用户、代码库或 CL 有所了解。所以提供并告诉他们更多的上下文。通常，您可以根据技术事实在自己和审查者之间达成一些共识。

解决冲突

解决冲突的第一步应该是尝试与审查者达成共识。 如果您无法达成共识，请参阅“代码审查标准”，该标准提供了在这种情况下遵循的原则。