代码审查的主要目的是确保逐步改善产品代码库的整体健康状况。代码审查的所有工具和流程都是为此而设计的。
为了实现此目标,必须做出一系列权衡。
首先,开发人员必须能够对任务进行改进。如果开发者从未向代码库提交过代码,那么代码库的改进也就无从谈起。此外,如果审核人员对代码吹毛求疵,那么开发人员以后也很难再做出改进。
另外,审查者有责任确保随着时间的推移,CL 的质量不会使代码库的整体健康状况下降。这可能很棘手,因为通常情况下,代码库健康状况会随着时间的而下降,特别是在对团队有严格的时间要求时,团队往往会采取捷径来达成他们的目标。
此外,审查者应对正在审核的代码负责并拥有所有权。审查者希望确保代码库保持一致、可维护及 Code Review 要点中所提及的所有其他内容。
因此,我们将以下规则作为 Code Review 中期望的标准:
一般来说,审核人员应该倾向于批准 CL,只要 CL 确实可以提高系统的整体代码健康状态,即使 CL 并不完美。
这是所有 Code Review 指南中的高级原则。
当然,也有一些限制。例如,如果 CL 添加了审查者认为系统中不需要的功能,那么即使代码设计良好,审查者依然可以拒绝批准它。
此处有一个关键点就是没有“完美”的代码,只有更好的代码。审查者不该要求开发者在批准程序前仔细清理、润色 CL 每个角落。相反,审查者应该在变更的重要性与取得进展之间取得平衡。审查者不应该追求完美,而应是追求持续改进。不要因为一个 CL不是“完美的”,就将可以提高系统的可维护性、可读性和可理解性的 CL 延迟数天或数周才批准。
审核者应该随时在可以改善的地方留下审核评论,但如果评论不是很重要,请在评论语句前加上“Nit:”之类的内容,让开发者知道这条评论是用来指出可以润色的地方,而他们可以选择是否忽略。
注意:本文档中没有任何内容证明检查 CL 肯定会使系统的整体代码健康状况恶化。你会做这种事情应该只有在紧急情况时。
指导
代码审查具有向开发人员传授语言、框架或通用软件设计原则新内容的重要功能。留下评论可以帮助开发人员学习新东西,这总归是很好的。分享知识是随着长年累月改善系统代码健康状况的一部分。请记住,如果你的评论纯粹是教育性的,且对于本文档中描述的标准并不重要,请在其前面添加“Nit:”或以其他方式表明作者不必在此 CL 中解决它。
原则
- 基于技术事实和数据否决意见和个人偏好。
- 关于代码风格问题,风格指南是绝对权威。任何不在风格指南中的纯粹风格点(例如空白等)都是个人偏好的问题。代码风格应该与风格指南中的一致。如果没有以前的风格,请接受作者的风格。
- 软件设计方面几乎不是纯粹的风格或个人偏好问题。软件设计基于基本原则且应该权衡这些原则,而不仅仅是个人意见。有时候会有多种有效的选择。如果作者可以证明(通过数据或基于可靠的工程原理)该方法同样有效,那么审查者应该接受作者的偏好。否则,就要取决于软件设计的标准原则。
- 如果没有其他适用规则,则审查者可以要求作者与当前代码库中的内容保持一致,只要不恶化系统的整体代码健康状况即可。
解决冲突
如果在代码审查过程中有任何冲突,第一步应该始终是开发人员和审查者根据本文档中的 CL 开发者指南和审查者指南达成共识。
当达成共识变得特别困难时,审阅者和开发者可以进行面对面的会议,或者有 VC 参与调停,而不仅仅是试着通过代码审查评论来解决冲突。 (但是,如果你这样做了,请确保在 CL 的评论中记录讨论结果,以供将来的读者使用。)
如果这样还不能解决问题,那么解决该问题最常用方法是将问题升级。通常是将问题升级为更广泛的团队讨论,有一个 TL 权衡,要求维护人员对代码作出决定,或要求工程经理的帮助。 不要因为 CL 的开发者和审查者不能达成一致,就让 CL 在那里卡壳。