代码审核标准
代码审核的核心目的是保证谷歌代码在不断的改进发展过程中还能持续保证健康。所有代码审核的流程与工具都是设计用于确保这个目标。
为了实现这个目标,我们做了很多的权衡。
首先,研发人员必须能够在个人的任务上做出改进。如果你从不提交代码的改进,那产品就无法提升。同样的,如果代码审核者对于任何变更提交都设置很高的门槛,也会影响开发者今后也提交改进的热情。
从另一个方面说,代码审核者的责任是要确保每个变更清单(CL)[1]的内容都能够在全局上保证代码质量,而不会随着时间导致质量不断变差。这样实际上是比较困难的,整体代码的质量削弱实际上是由于代码健康度中很小的问题不断累积引发的,特别当一个小组在明确工期的压力之下,需要走一些捷径才能够达成目标。
审核者对于他们审核的代码是有权利和责任的。他们需要确保代码基线是一致风格,可维护,以及在《代码审核应该审核什么》一文中提到的其他内容。
所以,我们期望代码审核中包含如下标准规则:
通常情况下,审核者需要确认一个变更清单对于整体系统代码一定是有质量提升的,哪怕目前还不是完美的。
在代码审核的指导规则中,这是最重要的原则。
当然,这条规则存在很多的限制。例如变更清单包含了一个系统不允许加入的特性,即使代码的设计和质量很好审核者也可以直接拒绝。
有一个很重要的观念,“完美”的代码是不存在的,只会有更好的代码。审核者不应该要求在接受提交前,就要提交者打磨变更清单中的每个小细节。审核者需要在系统持续改进和建议重要变更之间做出平衡。与其要求完美,审核者更应该重视的是持续改进。当一个变更清单对于整体系统的可维护性,可读性,可理解性都有改进时,就不应该因为“不完美”而被拒绝几天或者几周。
审核者应该一直都能自由的对可以改进的地方留下注释意见,但是如果这并不是很重要,可以在前面加上前缀如“Nit:”这样能让提交者知道这只是他们能够忽略的一个小优化点。
注意:本文档中的不会为任何恶化代码健康的提交做辩解,只有在特定的经济情况下你可以考虑这么做。
导师制
代码审核者有一个很重要的作用就是指导开发者关于语言、框架、或者软件设计的通用规则的一些新知识。能够留下一些评论帮助开发者学习一些新知识总是好事情。分享知识对于代码质量改进也是一件好事情。你需要谨记,如果你的评论是处于纯粹教育的目的,而不是本文档中提到的评判标准,请加上“Nit:”的前缀或者提示提交者这些在变更清单中并非需要强制修改。
原则
- 专业的现实情况与数据否定意见和个人偏好。
- 针对样式问题,《样式指南》是绝对权威。任何纯粹的样式观点(如空格等)在样式指南中没有提及的都是个人偏好。这部分的风格也应当统一,如果没有事先约定则接收提交者的样式。
- 软件设计的方面几乎不可能会是纯粹的样式问题或者个人偏好。 这些都是基于根本性的原则构建的,应当倚重于这些原则,而不是个人意见。有时会存在一些有限的选项。如果提交者能够证明(通过数据或者基于实际的工程原则)这些做法都是相近的,审核者应当接受提交者的偏好。否则就要取决于软件设计的标准规则。
- 如果没有其他可应用的规则,审核者应当要求提交者与现有的代码基线保持一致,只要变更内容不会影响到系统的健康程度。
解决冲突
面对代码审核中的任何冲突,第一步永远应该是开发者和审核者达成一致,原则就是基于本文档和其他的文档如《变更提交者指南》和《审核者指南》。
当很难达成一致时,提交者和审核者面对面的沟通或者语音沟通就很有必要,而不是通过代码审核的评论来解决冲突。(但是记住可以将讨论的结果以评论的形式记录在变更提交中,以便将来方便查阅)
如果上面的方法还是不能解决冲突,常见的方法就是升级问题。升级的方法通常是发起团队会议,要求技术领导参与其中,征询代码维护者或者管理者的意见,最终做出决策。不要因为提交者和审核者无法达成一致而闲置一份提交。
下一篇预告:代码审核时,我们应当寻找什么内容。
总结:
谷歌的代码审核中,首先要建立的是原则,编码样式、软件设计等等的原则。基于这些原则才能判定系统代码的健康程度,不是以个人喜好或者使用了某种框架。
审核者有拒绝提交的权利,但是背负着系统改进提升和维持健康程度的责任。要熟悉企业的各种原则产生自己的判断,并且将自己的意见分为必须和建议两种等级。
面对冲突时,不应当是提交者或者审核者某一方面的责任,而是双方应当据理力争尝试说服对方,当相互无法说服时可以升级问题,请求上级或者外部来评判。
在这些里面,所有的角色都有一个很重要的原则:真正的尊重代码,热爱代码。而不是随便糊弄任务,随便提交,随便审核,随便通过并且合并。这个要靠团队的文化和企业的健康来支撑。
-
CL: 是 "changelist" 的缩写,指提交到版本控制或者提交给代码审核的一个完整自包含的变更。 在其他组织中(非股改革)称之为变更(change)或者补丁(patch)。 ↩