小型 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,以便他们收到有关即将发生的事情的警告。在这种情况下,做好完成审查过程需要很长一段时间的准备,对不引入错误保持警惕,并且在编写测试时要更下功夫。

下一篇:如何处理审查者评论