Code Review 质量思考：解法

原创寻壑大淘宝前端技术

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

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

本篇主要基于第一篇文章提到的 CR 质量问题进行分析，并结合业内的相关实践提出相应的解法。

背景

从上篇文章的分析可以看出，DEF 目前整体的 CR 质量还有比较大的提升空间，主要表现在以下几点：

用于 CR 的时间偏短，导致评审速度过快，”秒过“型 CR 偏多；
CR 目前的评论功能及上层流程的串联对解决评审问题的帮助有限，导致评论率很低。
大尺寸 CR（超过 400 行）、异常 CR（超过 2000 行）等低评审质量情况偏多；

关于 CR 评审时间

发起评审时机

按照 DEF CR 目前的流程设计，我们或许可以尝试得出 CR 评审时间偏短的答案：

DEF 的流程设计围绕着开发和上线的流程展开，按照一般用户的使用流程，开发者会在日常发布和日常验证环境不断循环，直到验证通过，决定发布线上，在上线前需要让评审人评审已经完成的代码。此时会有两个问题：

CR 在上线前才提出，此时用于评审的时长受到一定影响
假设此时评审人指出了代码的问题，那么需要重新回去走日常发布和日常验证的循环，此时提出问题和解决问题的成本是相对比较高的

所以更好的 CR 流程应该和日常发布及验证的流程是基本并行的，我们在开发过程中就可以实时预览我们的改动，在感觉到已经完成某一个节点时，就可以提交一次阶段性的 CR 了，这样可以更前置的去开始我们的评审流程，并得到反馈。这个是我们的 分阶段代码评审 的一个设想，后文会有更详细的讨论。

评审时效性

考虑如下的一个日常场景：开发者在周一完成了日常的开发和验证，提交了 CR，准备在周四下午发布。收到评审邮件和钉钉提醒之后，评审人一般不会放下手头的工作，立即开始评审，然后等着等着就忘了自己有这个任务了。最后开发者在周四准备发布的时候，发现代码还没有被 CR，只好线下钉钉联系评审人开始评审流程。由于此时面临开发者发布时间的压力，评审人不得不加快脚步，一不留神就”秒过“了。

就这一个场景而言，我们可以借鉴集团审批流（bpms）的做法，尽量减少需要开发者线下催促的情况，降低沟通成本：

提供一个个人维度的任务中心（CR 列表），每天可以去定时处理指派给自己的任务
平台方去承接这个任务截止时间的控制和提醒工作（定时 & 倒计时通知，同时 DEF 研发平台增加站内消息功能），避免需要开发者线下沟通

为了保证通知的触达率，最基础的，CR 应该提供评审的邮件通知，保证用户可以感知到发给自己的 CR 消息；考虑到现在邮件的泛滥，CR 可以进一步提供钉钉通知的选项；对于那些日常迭代非常多非常快，需要多人协作的项目，可以提供钉钉群机器人的接入方式，让 CR 的创建、更新、评论回复等任何动态可以最快的被感知到。

通过这一套机制，理论上我们可以比较好的保证充足的评审时间，改善秒过的问题。

非变更区域折叠

还有一个问题是，目前的 IDE 视图会展示所有的旧版文件和新版文件的代码，通过对比得到 Diff 信息，考虑到留给一次评审的时间是有限的，CR 工具可以将此次非变更区域的代码默认进行折叠，避免在定位变更区域上占用宝贵的评审时间。

关于 CR 的反馈信息问题

提升评论的重要性

目前 DEF CR 的评论能力比较基础，评审双方可以就某一行代码进行评论，可以对评论进行回复。界面上我们会有一个简单的评论列表，通过树的形式来展示评论和回复的关系，如下图：

评论和回复功能本身是 CR 非常核心的能力，按照业界共识的对 CR 效果的预期，CR 是要作为一个一起解决问题的环节来发挥作用的。评论和评论反馈（提交新代码或者回复）要能成为一个小的闭环流程不断循环，最终这个循环是要以整个 CR 对开发者和评审人来说都没有新的问题来结束的，所以就这一点而言，平台需要提供一个 CR 评论的缺陷标注能力：评审人可以标记评论为一个缺陷，开发者通过提交新的代码或者回应问题来解决这个缺陷；除非所有的缺陷被解决，否则评审始终会被判定为未通过。这个能力的提供可以显著提升评论的重要性，开发者必须去关注评论的内容并解决，回归 CR 发现问题、解决问题的初衷，提升 CR 审核、对焦代码的价值。

[1] Substantially, they found an agreement in terms of a quick, lightweight process (CP1,CP2,CP3) with few people involved (CP4) who conduct group problem solving (CP5).[2] These tools support the logistics of the review process: (1) The author of a patch submits it to the code review tool, (2) the reviewers can see the diff of the proposed code change and (3) can start a threaded discussion on specific lines with the author and other reviewers, and then (4) the author can propose modifications to address reviewers’ comments. This feedback cycle continues until everybody is satisfied or the patch is discarded.

反馈方式的多样性

研究表明，CR 中评论信息的用词和情绪表达会对评审参与者的感受和评论的有效性有非常强的影响，一个好的 CR 应该在聚焦解决问题的同时，让评审双方能够有更好的感受。基于此，类似于 Github 的开源社区会支持在 CR 的评论下面打 emoji，在评审通过的时候支持在 全局评论 回复一句：LGTM（look good to me），这些都是一些让评审参与者感受非常好的点，鼓励大家能够更多的发表友好且有用的观点（下图是一条在 Github 的全局评论，可以给人非常好的正向反馈）。

Finally, although majority (51%) of the comments had‘Extremely Negative’ tones, only 57% of those comments were useful. On the other hand, comments with ‘Neutral’ or ‘Somewhat Negative’ tones were more likely to be ‘Useful’ (≈ 79% were considered useful).

关于 CR 的代码尺寸问题

DEF CR 的代码变更中位数目前处于一个相对健康的水平，在 40 行左右。但是有接近 20% 的 CR 是会导致 CR 质量急剧下降的大尺寸 CR，其代码变更行数超过了 400 行。甚至有接近 10% 的 CR 超过了 2000 行，基本上已经是不可能被正常 CR 的变更规模。导致 CR 尺寸偏大的原因无外乎两个：

没有启用 DEF 的多变更集成能力，一个发布迭代只有一个变更，但却包含了本次需要发布的所有功能，导致 CR 尺寸偏大
变更已经是功能的最细粒度，但是功能的复杂度导致了 CR 尺寸偏大

对于问题 1，目前 DEF 正在全面推广多变更集成模式，预计在近期所有的应用都将可以享受到多变更模式带来的迭代变更拆分能力，把一个迭代需要上线的功能拆分到对应的变更内，可以让协作更加轻松，CR 更加专注。

对于问题 2，业界一般的解法有以下两种：

开发者按照 commit message 的规范，主动将各个阶段的实现拆分到不同的 commit 内，评审人按照 commit 的粒度去查看
在最后提交评审的时候，基于机器学习做智能化的评审拆分，将变更内容拆分到不同的 CR 内

第一种解法是很直观的，一个成熟的开发者应当遵守比较好的 commit 规范：不把无关的改动杂糅在一个 commit 内，应当书写规范且达意的 commit message。在开发者严格遵守规范的前提下，工具还应该具备 commit 级别的版本对比能力。由于 commit 是很容易变的、不稳定的，基于 commit 的对比对工具的后端实现要求会非常高，需要具有实时对比的能力，查询压力也会比较大。目前经济体内的 aonecode 和 antcode 提供的均是基于提交版本（push）的对比能力，实现上只要在代码提交时将相关数据落库即可，暂不支持 commit 级别的对比。

第二种解法目前还没有比较成功的应用案例，只是学术界有一些相关的研究，如：《Helping Developers Help Themselves: Automatic Decomposition of Code Review Changesets》和《Untangling Fine-Grained Code Changes》。aonecode FY21 在 Java 场景基于语义拆分的方案尝试提供过一版大评审智能拆分能力，灰度测试后，考虑到实际的效果和后续投入的成本问题，暂时下线了。不过相信对于相对垂直的场景而言，比如确定框架（ice、rax等）的前端项目，机器学习应该可以取得比较好的拆分效果，未来可以是一个值得发力的方向。

那么除了机器学习以外，有没有工程的手段来解决这个问题呢？我们可以尝试推导一下：

如图所示，基于解法 1 的思想，我们可以尝试以 push 版本为节点代替 commit 节点，对评审进行划分，同时由于 push 是一个线上的流程，我们就可以在流程上通过卡口、提醒等予以保障。开发者在通过一系列 commit 完成一个小的阶段后，就可以及时的推送代码并提交一次阶段化评审，如此循环直到完成所有的代码开发，评审人进来时就可以按照开发者提交的阶段按序评审，甚至可以提前评审一个完成的阶段，并且可以看到开发者标注的每个评审大概的内容。上述流程是理想情况下的推导，实际落地的时候还是有不少需要考虑的问题的，比如评审人不会非常准时的就去评审你的代码，评审时代码已经更新了等等。我们下一篇文章会更详细的介绍我们的 分阶段代码评审。