CodeReview(CR)的本质是什么?是为了查错吗?还是为了KPI?本文分享阿里巴巴资深技术专家的观点:CR是一种长期的社会学行为和组织文化,通过CR,形成良性互动的技术氛围,传播和分享知识,提高代码质量,给出7条改善CR的实用建议效率和质量。关于代码审查(CodeReview)的文章很多,代码审查在很多组织中已经是一种标准化的做法。然而,很多团队在尝试代码审查实践时,都会产生以下疑问:“政治正确”的代码审查活动是否达到了预期的实际效果?我已经得到了很多代码,我应该从哪里开始呢?我应该审查哪些方面?我不应该做什么?我的代码是否正在被其他人认真审查?我怎样才能让其他人更容易审查代码?无非是两个方面:理解codereview的核心目标,树立正确的codereview预期。了解为什么代码审查可能不起作用,并采取有针对性的做法来提高代码审查的有效性。为什么要做codereview很多同学认为codereview是用来检查错误的,甚至希望用代码缺陷的数量来检验codereview的效果。这低估了代码审查的价值。代码审查最本质的功能不是发现问题。除了codereview,我们还有更多更好的发现问题的手段。代码审查的作用更多是社会学,一种长期的行为和组织文化。CR是代码规范的保障Coder视角:良性的社会压力你正在紧张地编码,交期迫在眉睫。您的组织对代码单元测试有要求:所有新代码都必须进行完整的自动化单元测试。但是迫于压力,想对自己降低要求,单元测试这部分就不要写了,以后再写,或者为了满足工具的覆盖要求,先写一点但是可以带来覆盖率测试(例如没有断言的测试)。但是,一旦你想到你的代码会被发送给你的同事审查,你是不是对刚才的这个想法有点压力呢?这种压力是良性的,能给你带来即时的反馈。防止您选择短期获利、长期亏损的“投机”行为。如果没有codereview环节,也许你真的会“为所欲为”,但最终还是要为这种刁钻的行为买单。Maintainer的观点:代码可读性保证有多种方法可以实现相同的软件需求。有兴趣的读者可以自行搜索《HelloWorld的N种写法》。虽然条条大路通罗马,但不同的路,成本是不一样的。小到变量命名,大到设计结构,如果采用不常见的做法,往往是后续代码维护者的坑。此维护活动可能会在1个月后、1年后甚至更长时间后发生。甚至那个时候,作为作者的你已经不在这个团队里了,也没有人能理解当时为什么要这样设计软件。代码审查提前强制这个反馈周期。代码写好后,马上就会有一个或多个读者——他们就是这段代码的Reviewer。因此,此代码在编码后立即进行了可读性测试。更理想的是,如果组织已经有了编码指南和设计指南,它还可以确保代码遵循这些指南。如果在CR过程中发现这段代码不符合规范,那是好事,它指向了CR的另一个关键价值:知识传播。CR带来知识传播和设计共识。你可能是一个团队的领导者,正在为如何提高团队成员的编程能力而发愁。你想让他们看,所以你介绍了《整洁代码》这样的入门书籍,你也介绍了经典《设计模式》,你还推荐了《领域驱动设计》。您还希望团队成员了解产品的业务逻辑,因此您希望团队成员定期共享业务。所有这些努力都是好的。但是,您也有可能被击中。一个月过去了,团队成员似乎对命名约定有了一个概念,但对于如何命名还没有达成共识。很多同学已经了解了一些设计模式,但有些人过度使用模式,使代码臃肿,而有些人只知道单例。团队成员为什么是实体对象,什么是值对象正在热议。聚合是什么,应该在什么场景下使用,谁也说不清楚。你真正缺少的,是一个场景。抽象概念不落入具体事物,就很难形成共识。有人可能知道海洋法体系的“判例”,这是在法律层面形成共识的一个很好的方式。codereview其实就是形成一个case:哪种设计合理,哪种设计不合理。通过具体问题的分析,团队会逐渐形成设计共识。在这个过程中,对这些共识不太熟悉的新生也可以慢慢融入。验证逻辑正确性当然,CR也可以用来验证逻辑正确性。确保代码逻辑正确是设计者的责任。为了防止CR被滥用和期望过高,我们先在这里申明一点:保证代码逻辑正确是设计者的责任。发生空指针错误。是因为糟糕的编码、糟糕的CR还是糟糕的测试?首先,这个问题一定是代码的作者制造的。把这个板放在审稿人上公平吗?或许,Reviewer确实有责任发现这样的问题,但是如果代码有很多错误怎么办?一次review1000行代码,根本看不出来怎么办?我可以找到很多理由来解释为什么漏掉了这样一个空指针错误。查找逻辑错误的其他方法你有很多其他的方法来查找错误,它们通常并不昂贵,例如:编写自动化单元测试使用代码静态检查工具以上两种都是专业的,无论CR活动开发人员和开发组织应该采取的行为大力推广。找错的价值在认识到以上两点的前提下,codereview确实应该用来找错——它本质上是建立了一个冗余机制,通过多人对同一段代码的工作,可以发现可能代码中的错误。发生的认知错误(个别开发人员通常很难发现)和疏忽。高效高质量的代码审查哪些因素阻碍了代码审查的有效性代码审查本身并不难,但如果考虑以下因素,可能会更复杂:你可能对你所在的代码的设计上下文一无所知审查。可能很忙,一下子收到几千行代码需要review……实践建议建议一:小批量——每次review的代码量要小。研究发现,成功的CR活动必须是小规模的。例如,《Modern Code Review:A Case at Google》论文称,在谷歌的CR活动中,35%的CRs只修改了一个文件,90%的CRs修改了不到10个文件,甚至有10%的CRs只修改了1个文件。代码行。代码量少的好处很明显:很清楚哪里修改了,问题一目了然。一次把1000+行代码推给别人,还想得到有价值的Review是不太可能的。当然,Review所代表的功能应该是有意义的、完整的。如果不是修复缺陷,这也在一定程度上对开发者迭代开发功能的能力提出了要求。建议2:multi-batch——小batch频繁出现的review,必然会出现multi-batch。2013年微软的一篇论文《iExpectations, Outcomes, and Challenges Of Modern Code Review》和前述的谷歌论文中都提到了频繁review的做法。其中,谷歌每个Developer每周的代码更改次数中位数是3次,每个Reviewer每周Review次数的中位数是4次。建议3:找到合适的人——适合审查你代码的合适的Reviewer?选择与正在审查的代码无关的人绝对是不明智的。下面列出了一些潜在的候选人:如果您的组织有所有者机制,那么所有者应该是合适的人选。与您在相同背景下工作的同事。最近修改了相同代码的同事。比你资深的程序员,希望得到他们专业的反馈,现在有一些工具可以根据上下文推荐Reviewer,也方便选择合适的Reviewer。建议四:快速响应当每次review的粒度小,review频繁的时候,快速响应是可能的,也是必然的要求。在这个数据上,谷歌的中位数是4小时。这个指标可以作为一个很好的参考。建议5:使用现代工具快速响应、高质量审核离不开现代工具。现代审阅工具可以自动集成到工作流程中,突出显示更改,甚至自动总结更改。这方面有很多现代工具可用,还没有选择工具的读者可以自行搜索。建议六:考虑结对编程当我们提到“小批量、多批次、快速反馈”时,有过结对编程经验的同学马上就会反映,结对编程和CR很像。在结对编程中,一起编程的两个同学有着完全相同的上下文,没有上下文切换的烦恼,没有时间不够的烦恼,不需要额外的工具,随时随地反馈。事实上,在我看来,结对编程是最好的代码审查。建议七:线上复习与线下复习相结合线上复习应该是一种正常的行为。考虑到CR的“知识传播”价值,线下复习是一个有益的补充。有经验的团队会定期或不定期地组织线下点评,相比线上点评可以获得更广泛的知识传播,也能引发更多的热议和争论,有助于形成更高质量的共识。数字指标研发的全面数字化带来了一些有价值的数据见解。如果有工具支持,可以通过一些指标的观察,不断推动CR活动。我们总结了一些建议的指标和数据如下:从Author的角度:单次review的代码行数(主要指标)Reviews的频率(参考)从Reviewer的角度:Responsetime评论数和reject数rate从Reviewer群体的角度,也可以发现Author-Reviewer之间的社区关系,这对于理解知识扩散也是一个有价值的信息。不该做的事:避免使用CR的数字指标来衡量,即使是出于好意。CR的本质是文化建设。强烈建议只将CR指标作为改进的指导,而不要用于性能相关的评价,无论是前面提到的指标,还是与Review质量甚至缺陷相关的数据。实践您组织的代码质量和技术氛围如何?你在实施代码审查吗?如果没有,你愿意尝试一下吗?如果是这样,现有的做法是否符合代码审查的真正目标?
