DEF (阿里巴巴前端研发平台)在 FY21 基于 Aone (阿里巴巴代码托管平台)能力和 KAITIAN 纯前端版本搭建了一套 CodeReview 系统,致力于面向前端提供更好的 CodeReview 体验,建立起更符合前端特点的质量评价体系。《CR 质量思考》系列文章共分为三个部分:
本篇主要介绍 CodeReview 的历史发展和质量评价指标,并通过 CodeCollaborator 的报告介绍了 CodeReview 质量数据的分析方式,最终对比 DEF 目前的数据来了解前端目前的 CodeReview 质量现状。
CodeReview (后文简称 CR)是一个在软件开发流程中被广泛应用的步骤。通过 CR,我们可以实现以下目的:
传统的 CR 一般是线下进行的,一群人拉一个会,对着某一段代码进行评审。这种流程相对来说比较重,很费时间,而且是一个同步的流程(heavyweight process)。而现阶段的 CR 一般都是线上化、工具化的,通过异步的方式进行评审(lightweight process),会更加轻量。将传统的线下流程搬到线上后,对应的 CR 流程变成了这样:
CR 流程工具化之后,很多传统 CR 模式的问题被解决了,CR 作为一道非常重要的质量保障的防线,开始被不同的软件研发流程所广泛采用。然而在实际使用的过程中,CR 形式化的问题开始逐渐显现,大尺寸的代码变更、发布窗口的时限要求、工具的可定制性等等问题,让很多评审人在评审时只是走马观花,偏离了 CR 的初衷。
那么应该如何去建立 CR 质量的度量体系,从而针对性的解决 CR 过程中影响质量的关键问题呢?首先我们关注一下 CR 流程中可以收集到的原始数据信息:
Although we agree that sLOC (source code ignoring whitespace) is better correlated with “executable code” than is LOC, in our experience the comments often have as much to do with the review as the source code. Many defects are found in document/code mismatch or just in lack of proper documentation. Therefore we believe LOC is a better measure of the amount of work necessary for the reviewer.
基于这些原始指标,我们可以进一步得到下面这些分析指标:
基于这些数据指标,业内做了很多相关的研究,我们以 CodeCollaborator 的分析报告为例,了解一下 CR 质量的度量方式。
早在 2006 年,SmartBear Software 就组织了一次长达 10 个月,涵盖 2500 个评审,涉及 320 万行代码,共有 50 个开发者参与的关于代码评审的研究活动。这也是历史上最大规模的关于轻量评审流程的研究。在此之前,基本上所有的文献都是关于传统的线下评审,而轻量评审的有效性以及如何提升有效性的准则还没有相关的研究。SmartBear 的这一研究基于他们旗下的 CR 工具 CodeCollaborator 展开。CodeCollaborator 很好的实践了轻量评审的流程,一次代码变更提交到代码仓库的流程如下:
CodeCollaborator 采集到的 CR 数据点的分布如图左所示(横轴是待评审的代码行数,纵轴是评审速度,使用双对数坐标系绘制),在裁剪掉图中那些明显不合理的 CR 数据之后(评审速度大于 1500 行/小时、评审时长小于 30s 或评审代码大于 2000 行,不合理数据约占 21%),我们以普通坐标系绘制得到图右。CodeCollaborator 发现大部分的评审在 200 行以内,大部分评审速度在 500 行/小时以下:
得到合理范围的评审时间和评审行数的数据之后,我们还需要收集 CR 的关键质量衡量指标:缺陷数。CodeCollaborator 的缺陷定义为:
When a reviewer or consensus of reviewers determines that code must be changed before it is acceptable, it is a “defect.” 当有评审人或所有评审人都认为这处代码必须要修改才能被接受时,这处代码就是一个“缺陷”。
CodeCollaborator 提供了一个标注代码缺陷的能力(设置评论为缺陷),还支持设置缺陷的严重程度,这个功能让我们可以获取到一部分缺陷的数据。考虑到有相当一部分开发者只会使用评论能力交流,但是不会主动标注缺陷,因此除了使用系统上的缺陷数据之外,研究者还去研究了 300 个样本的数据来人工标注缺陷,最终得到了较能反映真实情况的缺陷数据。
假设代码库的理论缺陷密度是一个常量,实际评审发现的缺陷密度可以很有效的反馈出代码评审的质量,CodeCollaborator 就缺陷密度与代码行数、缺陷密度与评审速度以及缺陷密度与开发者是否预审查的关系进行了分析,如下图所示:
从上述的图表可以得出以下结论:
缺陷密度和代码行数、评审速度息息相关,那么缺陷找出的速度,也就是 CR 的效率是否和这些因素有关联呢?答案是否定的,在正常的 CR 行为内,除了少数小尺寸的代码评审(6%)可以达到比较高的水平,缺陷找出的速度基本上是一个恒定的值,一般是在 15 个/小时 左右。
DEF CR 目前记录了比较完善的评审信息数据,包括 CR 代码行数、评审时长和评论数等等。就目前统计的数据而言(2021-6-30 整体数据),整体的评审质量还是有较大的提升空间的,主要体现在以下几点:
作为对照,Google 的 CR 质量数据为:
[1] The median change size in OSS projects varies from 11 to 32 lines changed, depending on the project. At companies, this change size is typically larger, sometimes as high as 263 lines. We find that change size at Google more closely matches OSS: most changes are small.
[2] We attempt to quantify how “lightweight” reviews are (CP1). We measure how much back-and-forth there is in a review, by examining how many times a change author mails out a set of comments that resolve previously unresolved comments. We make the assumption that one iteration corresponds to one instance of the author resolving some comments; zero iterations means that the author can commit immediately. We find that over 80% of all changes involve at most one iteration of resolving comments.
DEF 的 CR 质量目前还在一个比较一般的水平,按照目前的流程设计,CR 往往会卡在日常验证完成后、线上发布前提交,压缩了评审人的评审时间;一个 CR 对应一个变更,对于还没有开启多变更集成的应用,一个发布分支会包含本发布周期需要发布的全部变更内容,CR 的变更行数会变得很大。DEF 整体 CR 质量,特别是评审速度这一块还有相当大的提升空间,所以下一阶段应该如何解决这个问题,是一个非常值得思考的命题。
引用文章:
《Best Kept Secrets of Peer Code Review》:https://static1.smartbear.co/smartbear/media/pdfs/best-kept-secrets-of-peer-code-review_redirected.pdf
《Modern Code Review: A Case Study at Google》:https://research.google/pubs/pub47025/
往期推荐
VS Code 是如何优化启动性能的?