DEF （阿里巴巴前端研发平台）在 FY21 基于 Aone （阿里巴巴代码托管平台）能力和 KAITIAN 纯前端版本搭建了一套 CodeReview 系统，致力于面向前端提供更好的 CodeReview 体验，建立起更符合前端特点的质量评价体系。《CR 质量思考》系列文章共分为三个部分：

• CR 质量思考：指标和现状
• CR 质量思考：解法
• CR 质量思考：分阶段评审实践

本篇主要介绍 CodeReview 的历史发展和质量评价指标，并通过 CodeCollaborator 的报告介绍了 CodeReview 质量数据的分析方式，最终对比 DEF 目前的数据来了解前端目前的 CodeReview 质量现状。

背景

CodeReview （后文简称 CR）是一个在软件开发流程中被广泛应用的步骤。通过 CR，我们可以实现以下目的：

• 知识传递：作者或者评审人可以在评审的过程中传授或学习到知识
• 代码规范：评审可以保证代码风格是一致的
• 代码把关：评审时可以就代码的架构、可维护性、边界条件等展开交流
• 问题控制：发现逻辑漏洞、代码缺陷等质量问题

传统的 CR 一般是线下进行的，一群人拉一个会，对着某一段代码进行评审。这种流程相对来说比较重，很费时间，而且是一个同步的流程（heavyweight process）。而现阶段的 CR 一般都是线上化、工具化的，通过异步的方式进行评审（lightweight process），会更加轻量。将传统的线下流程搬到线上后，对应的 CR 流程变成了这样：

• 创建评审：完成代码开发后，创建一个评审
• 预审查：开发者自行预览改动及自动化的静态扫描等结果，保证代码对他自己来说没问题
• 评审人评论：评审人对改动进行评审，评论指出有问题的部分
• 处理评论：开发者处理那些需要回应的评论（可以通过回复或提交新代码来处理）
• 评审通过：评论和处理评论的循环结束后，一个或多个人评审通过

CR 流程工具化之后，很多传统 CR 模式的问题被解决了，CR 作为一道非常重要的质量保障的防线，开始被不同的软件研发流程所广泛采用。然而在实际使用的过程中，CR 形式化的问题开始逐渐显现，大尺寸的代码变更、发布窗口的时限要求、工具的可定制性等等问题，让很多评审人在评审时只是走马观花，偏离了 CR 的初衷。

CR 质量指标

那么应该如何去建立 CR 质量的度量体系，从而针对性的解决 CR 过程中影响质量的关键问题呢？首先我们关注一下 CR 流程中可以收集到的原始数据信息：

• 变更行数 LOC：一次评审过程中发生变更的总代码行数，包含文档、空白符等非可执行类的代码变更（经验表明，有很多 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.

• 评审时间 Inspection Time：一次评审时评审人的总阅读时间，通过文件打开、关闭的间隔来自动累加
• 缺陷数 Defect Count：评审人添加的需要解决的评论（部分评论纯做交流，不需要开发者解决），以及静态扫描工具报告的需要修复的部分（会作为一个特殊的评论添加到对应行上），定义为一个缺陷，缺陷数可以很好的衡量一次评审的最终效果

基于这些原始指标，我们可以进一步得到下面这些分析指标：

• 评审速度 Inspection Rate：评审的行数/评审时间得到，体现一个评审人理解和阅读代码变更的速度
• 缺陷发现速度 Defect Rate：缺陷数/评审时间，体现了评审的效率，缺陷发现速度与工具的能力息息相关，比如传统的线下评审的方式可能只有 5 个/小时，而轻量的异步线上工具可能可以达到 20 个/小时
• 缺陷密度 Defect Density：缺陷数/变更行数，缺陷密度体现了每千行代码的缺陷数量，对于给定的源码，可以很好地反应评审的有效性，但是对于不同的项目，该值差异会比较大，比如稳定的代码仓库的缺陷密度为 5 个/千行，而一个初级开发者提交的新代码可能会达到 100-200 个/千行

基于这些数据指标，业内做了很多相关的研究，我们以 CodeCollaborator 的分析报告为例，了解一下 CR 质量的度量方式。

CodeCollaborator 分析报告

早在 2006 年，SmartBear Software 就组织了一次长达 10 个月，涵盖 2500 个评审，涉及 320 万行代码，共有 50 个开发者参与的关于代码评审的研究活动。这也是历史上最大规模的关于轻量评审流程的研究。在此之前，基本上所有的文献都是关于传统的线下评审，而轻量评审的有效性以及如何提升有效性的准则还没有相关的研究。SmartBear 的这一研究基于他们旗下的 CR 工具 CodeCollaborator 展开。CodeCollaborator 很好的实践了轻量评审的流程，一次代码变更提交到代码仓库的流程如下：

1. 开发者通过命令行或 GUI 工具提交变更到 CR 系统
2. 开发者指定评审人，评审人会收到邮件通知
3. 评审人进入 CR Web 页面进行评审，半数的 CR 只有一个评审人
4. 评审中会看到代码的 Diff 信息，评审人可以在有问题的行进行评论，开发者可以和评审人在评论下展开讨论
5. 评论可以被标注为缺陷，开发者可以提交新的代码去修复缺陷，流程往复直到没有新的缺陷被提出后
6. 评审人评审通过，代码合并

数据准备

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 个样本的数据来人工标注缺陷，最终得到了较能反映真实情况的缺陷数据。

CR 有效性 - 缺陷密度分析

假设代码库的理论缺陷密度是一个常量，实际评审发现的缺陷密度可以很有效的反馈出代码评审的质量，CodeCollaborator 就缺陷密度与代码行数、缺陷密度与评审速度以及缺陷密度与开发者是否预审查的关系进行了分析，如下图所示：

从上述的图表可以得出以下结论：

• CR 应尽量控制在 200 行内，不能超过 400 行，超出了就会导致评审人无法掌控，缺陷无法被全部发现
• 评审速度在 300 行/小时会有最佳的缺陷找出效果，500 以上的速度就会导致有大量的缺陷被遗漏
• 会提前为评审做准备，写好注释和评论的作者在评审中的缺陷密度会大大降低，很多会降到 0，因为作者在尝试讲清楚自己的变更代码时（我会了和我会教别人了是两种境界），会主动发现代码里的问题并修复掉

CR 效率 - 缺陷找出速度分析

缺陷密度和代码行数、评审速度息息相关，那么缺陷找出的速度，也就是 CR 的效率是否和这些因素有关联呢？答案是否定的，在正常的 CR 行为内，除了少数小尺寸的代码评审（6%）可以达到比较高的水平，缺陷找出的速度基本上是一个恒定的值，一般是在 15 个/小时 左右。

DEF 的数据现状

DEF CR 目前记录了比较完善的评审信息数据，包括 CR 代码行数、评审时长和评论数等等。就目前统计的数据而言（2021-6-30 整体数据），整体的评审质量还是有较大的提升空间的，主要体现在以下几点：

• 有接近 20% 的 CR 超出了评审质量阈值 400 行，接近 10% 的 CR 超过正常数据 2000 行；
• 用于 CR 的时间偏短，导致评审速度过快（超过了评审质量速度阈值 500行/小时）；
• CR 能够给出的反馈信息非常有限，整体评论率很低。

作为对照，Google 的 CR 质量数据为：

• 评审代码行数中位数介于开源项目（一般为 11~32 行之间）和产业内部项目（一般低于 263 行）之间，更接近开源项目的尺寸
• 20% 的评审会有超过一次的缺陷提出和缺陷解决的过程

[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/