Google 是如何做 Code Review 的?| 原力计划( 八 )


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.
第一句描述了实际做了什么 。其余的描述解释了为什么要进行更改 , 并给了reviewer很多背景 。
在提交CL之前 , 请检查描述
在Review期间CL可能会经历重大改变 。在提交CL之前 , 有必要review CL描述 , 以确保描述仍然反映CL所做的事情 。
简短化的CL
为什么要写成一系列简短的CL?
简短的CL有这些好处:

  • Code Review更快 与比起花30分钟审查一个大型的CL相比 , 对审查者来说花5分钟审查一系列小的CL更加容易.
  • 审查更加彻底 。进行较大的更改后 , 审阅者和作者往往会因大量详细评论的来回移动而感到沮丧 , 有时这些评论会漏掉或遗漏重要的观点 。
  • 减少导致bug的可能性 。由于您所做的更改较少 , 因此您和您的审阅者更容易有效地推断出CL的影响 , 并查看是否导致bug 。
  • 减少不必要的工作 当你写了一个巨大的CL , 然后审查者觉得你总体方向错了 , 这会导致你浪费大量的工作 。
  • 更方便合并代码 因为大型的CL会导致大量的冲突 , 因此合并大型的CL会浪费很多时间 , 并且这将会是你经常做的工作 。
  • 有助于你作出更好的设计 优雅的设计并且完善一个小的改动比大的改动更加容易
  • 降低审查者的难度 提审部分改动 , 不会影响你继续编码 。
  • 回滚更容易 大型CL很有可能会在初始CL提交和回滚CL之间更新修改文件 , 从而使回滚变得复杂(中型CL也可能也需要回滚可能也会这样) 。
注意!审查者可以因为你的改动过于巨大直接拒绝掉你通常 , 他们会感谢您的贡献 , 但要求您以某种方式使它成为一系列较小的更改 。不管是你把这些改动拆分成小的改动 , 还是和审查者争论让他接受都会耗费你大量的时间 。但是编写小型改动就不会有这样的问题 。
怎么样算简短?
通常 , CL的正确大小是一个独立的更改 。这意味着:
  • CL所做的最小更改仅解决了一件事情 。通常 , 这只是功能的一部分 , 而不是一次完整的功能 。通常 , 宁可编写过小的CL也不要写太大的CL 。你可以和你的审查者商量找到合适的尺度 。
  • 【Google 是如何做 Code Review 的?| 原力计划】审阅者需要了解的有关CL的所有内容(将来的开发除外)都在CL , CL的描述 , 现有代码库或他们已经查看过的CL中 。
  • 检入CL后 , 系统将继续对其用户和开发人员正常运行 。
  • CL不够小的话会导致其难以理解 。如果你新增加了api , 那么在同一个CL应该包括这个api用到的方法 , 以便审查者更好的理解 。也能防止检入没有用的api 。
多大算大 , 没有一个明确的要求 。对于CL而言 , 100行通常是一个合理的大小 , 而1000行通常太大 , 但这取决于您的审阅者的判断 。更改分布的文件数量也会影响其“大小” 。可以在一个文件中进行200行的更改 , 但是将其分布在50个文件中通常会太大 。
请记住 , 尽管从您开始编写代码起就一直与您紧密联系 , 但审阅者通常没有上下文 。对您来说 , 看起来像可接受大小的CL可能会让您的审阅者不知所措 。毫无疑问 , 尽可能把CL些小是正确的 。审查者很少抱怨CL太小
什么时候大型的CL也是可以的?
在某些情况下 , 较大的更改没有那么糟糕: