翻译 HOW TO DO CODE REVIEW

原文链接:https://google.github.io/eng-practices/review/reviewer/

文中所写CL,是指change list,等价于MR,PR

THE STANDARD OF CODE REVIEW

code review的目的是为了提升codebase代码质量,为此必须要做一些权衡。

首先!dev必须能够在他们负责的任务上取得进展,如果reviewer让任何更改变得过于困难,那么开发人员就没有动力提MR

reviewer有责任确保每一条change list不会使codebase随着时间推移而变烂。做到这一条可能会比较困难,因为codebase的质量是随着一条一条的小的代码修改而逐步降低的,特别是当面临紧急任务目标需要实现时,一般都会采取最快的方式将代码合入。

此外,reviewer对自己正在review的代码有“所有权”和责任,需要确保codebase保持“代码一致性”/“代码可维护性”/以及link中的其他条目

因此,我们总结了下面的原则和标准,应该应用于google的codebase code review中。

总体来说,如果一个MR可以确定性的提高现在代码的健康度,reviewer都应该接受这个MR,即使MR还不是最完美的方案

当然,对上面这条表示还是有些需要补充的。比如,如果一个MR增加了一个reviewer并不想要的功能,那么reviewer应该拒绝MR,即使代码写的不错。

关键点是:没有绝对完美的代码,而只有相对更好的代码。reviewer不应该要求提MR的人把代码中的每一处都润色完,再merge代码。相反,reviewer应该权衡push进度的需求和代码中所建议的改动的重要性。reviewer应该追求连续性的代码提升,而不是寻求完美的提升。总的来说,一个CL提高了可维护性、可读性、codebase的易读性,都不应该推迟被合入,即便MR还不够完美。

reviewer可以随意表达对代码的建议,比如如何使MR变得更好。但是如果这个建议不重要的话,可以在comment前面加上"Nit:",让dev知道只是一个小的建议,但是无所谓。

只有在被push的情况下提交MR,才会破坏代码的健康度(原文 Note: Nothing in this document justifies checking in CLs that definitely worsen the overall code health of the system. The only time you would do that would be in an emergency.)

指导

code reivew有一个重要的功能:教给dev关于一种编程语言的新知识,一个框架或者一个通用的软件设计原则。写一些comment去教dev一些新的知识是挺好的事,在MR中分享知识也有利于提高codebase的健康度,如果你写的comment完全是有教育意义的,而与codebase的开发文档中所描述的MR相关内容无关的话,就写个"Nit:"

原则

技术和数据是凌驾于个人喜好和个人意见之上的。

关于代码的style问题,参考google的style guide就完全够了

任何style guide中没有提及的事情都是dev自己个人习惯的事情,不重要。

软件设计的各个方面不是style问题,也不是个人喜好问题,而是有一套基本原则的,如果dev通过数据或者是可靠的工程原理证明了有多种方法都是同样有效的,那么reviewer应该接受dev的个人喜好的选择。否则应该遵守标准的原则。

如果还没有一个成套的基本原则,那就遵守codebase中的原则就行。

关于code方面的意见冲突

在code review中的任何意见不一致,dev和review的第一步是达成共识,即根据文档内容link1link2
达成一致的意见。

当达成一致意见变得特别困难的时候,当面meeting或者线上会议一下或许很有利于达成一致意见,而不是只是通过MR中的comment来沟通。

如果这样都不能达成一致意见,那就只好把这个问题发到全组,邀请codebase的maintainer和全组来帮助决定,该如何解决。

即便dev和review意见不一致,也不能让MR一直挂在那里

What to look for in a code review

设计

CL中的设计是否有意义?这些修改是codebase中的,还是第三方lib中的?是否这些修改能很好的融入现在的系统/codebase?现在增加这个功能是否合适?

功能

posted @ 2023-01-01 13:29  ijpq  阅读(19)  评论(0编辑  收藏  举报