Code Reivew的目的是为了使所有代码的代码健康得到改善(代码健康是指代码的可维护性,可阅读性,稳定性及简洁性)。所有的工具,流程都是为此目的而设计的。
为了达到这一目的,我们必须权衡取舍。
首先,开发人员必须针对改善代码而有所行动,毕竟如果你从来不去改善代码,则代码基永远不会得到改善。同时,评审员如果使任何变更都很难以被批准 ,则开发者以后就不会去改善代码了。
另一方面,评审员有责任去保证每一次提交列表(CL)没有降低整个代码库的代码健康。这很棘手,因为代码库整体的健康程度的下降是由于经常性的一小部分代码质量下降积累而致的。特别是当一个团队在重大的时间限制下,他们觉得必须走捷径以实现他们的目标。
此外,评审员对他们审查的代码拥有所有权和责任。他们希望确保代码库保持一致、可维护,以及“Code Review应该Review什么”中提到的所有其他内容。
因此,我们得到了以下规则作为我们在代码审查中期望的标准:
一般来说,代码审查者应该通过那些绝对能提高代码健康的但是并不完美的提交列表(CL).
这是所有代码审查指南中的高级原则。
当然,这是有限制的。例如,如果CL添加了评审员不希望在其系统中使用的功能,那么即使代码设计良好,评审员也可以拒绝批准。
这里的一个关键点是,没有所谓的“完美”代码,只有更好的代码。评审员不应该要求作者在批准之前对CL的每一个微小部分进行打磨。相反,评审员应该权衡取得进展的需要和他们所建议的变更的重要性。评审员不应该追求完美,而应该追求持续改进。作为一个整体,提高系统的可维护性、可读性和可理解性的CL不应该因为它不是“完美的”而推迟几天或几周。
评审员总是可以自由地留下评论,表示某些东西可能会更好,但是如果它不是很重要,可以在前面加上“Nit:”这样的前缀,让作者知道这只是一种修饰,他们可以选择忽略。
注意:在本文档中,没有任何内容证明检入会明显地恶化系统的整体代码健康状况的CL是合理的。只有在“紧急情况”下你才会这么做。
指南
代码评审的一个重要功能是向开发人员传授有关语言、框架或一般软件设计原则的新知识。留下有助于开发人员学习新东西的评论总是好的。随着时间的推移,共享知识是改善系统代码健康状况的一部分。请记住,如果您的评论纯粹是教育性的,但对于满足本文档中描述的标准不是至关重要的,请在前面加上“Nit:”或以其他方式表明作者不必在本CL中解决它。
- 技术事实和数据压倒了意见和个人偏好。
- 在风格方面,风格指南是绝对权威。任何没有在样式指南中的纯样式点(空格等)都是个人偏好的问题。风格应该与现有的一致。如果没有以前的风格,那就接受作者的风格。
- 软件设计的各个方面几乎从来都不是纯粹的风格问题或个人偏好。它们是建立在基本原则基础上的,应该以这些原则为依据,而不是简单地以个人观点为依据。 有时有一些有效的选项。如果作者能够证明(通过数据或基于可靠的工程原理)几种方法是同样有效的,那么评审员应该接受作者的偏好。否则,选择取决于软件设计的标准原则。
- 如果没有其他规则适用,那么评审员可以要求作者与当前代码库中的内容保持一致,只要这不会恶化系统的整体代码健康状况。
解决分歧
在代码评审的任何分歧中,第一步总是让开发人员和评审员根据本文档的内容和CL作者指南和评审员指南中的其他文档达成一致。
当达成一致意见变得特别困难时,通过评审员和作者之间进行一次面对面的会议或视频会议来解决分歧,而不是试图通过Code Review的评论来解决。(如果您这样做了,请确保将讨论的结果记录在CL的评论中,以供将来的读者参考。)
如果这还不能解决问题,最常见的解决方法就是升级。通常情况下,升级的途径是更广泛的团队讨论,让TL参与进来,请求代码维护者做出决定,或者请求经理提供帮助。不要因为作者和评审员不能达成一致意见而让CL悬而未决。