如何有效地进行代码 Review？

左手的ㄟ右手 2022-12-18 11:54 249阅读 0赞

### **为什么要做代码 Review** ###

为什么要代码 Review，相信每个人心中都有比较一致的答案，Google 搜索一下也能找到一大堆的文章。这里简单总结几点：

**1）提高代码质量**

这是代码 Review 的初衷，也是代码 Review 最直接的价值。Reviewers 根据各自的经验，思考方式，看问题的角度给代码提出各种可能的改进意见，从而形成更好的代码以及产品质量。

我们知道产品问题越晚提出解决它的代价就越大，参与进去的人、要走的流程都会越来越多。代码 Review 可以说是早期解决问题最有效的途径之一了，在代码 Review 中解决掉哪怕一个小问题都能避免后续一堆的麻烦事。

**2）形成团队统一的高质量标准**

有效的代码 Review 最终会在团队范围内建立起统一的质量标准，并且会随着团队成员的互相学习和促进形成更高的标准。参与者会在代码 Review 的过程中基于具体问题从不同角度提出改进意见，并最终做出当前最佳的选择并形成共识。随着代码 Review 的有效进行，团队成员会有意识地关注代码质量，从而形成越来越高的事实上的质量标准。

**3）个人快速成长**

通过有效的代码 Review，Author 和 Reviewer 都将获得成长，在 Review 过程中参与人就具体的问题展开深入的讨论，无论是怎么写出整洁的代码、怎么更好地更全面地思考问题、怎么最佳地解决问题，参与人都可以互相取长补短，互相提高。通过具体代码实现进行的讨论往往是最深入和有效的，代码 Review 是开发者提高代码能力最重要的途径之一。

有的人认为代码 Review 就是 Reviewer 帮助查找代码中的 Bug，**其实代码 Review 不应该是一个单向的过程，而应该是个双向交流的过程**，Reviewer 帮助 Author 提出代码改进意见的同时，也是向优秀的 Author 学习的过程。我们都知道提高代码能力一个有效的途径是阅读优秀的项目代码，但是阅读项目代码有着不小的难度，这也是大部分人没有去执行的原因，而 Review 资深同事的代码，我们在阅读代码的同时能够直接进行有效的沟通，这是一个快速有效的学习途径，尤其对开发经验还不算丰富的开发者而言。

### **如何做好代码 Review** ###

### **2.1. 什么时候发起 Review** ###

在代码 Review 上，Author 需要意识到：**Reviewer 的时间是昂贵的**。因此在正式邀请 Reviewer 发起代码 Review 前，Author 有几项需要注意的点，这些都能提高代码 Review 的效率，节省 Reviewer 的时间。

**2.1.1. MR (Merge Request)**

也称为 PR（Pull Request）， MR 是我们进行代码 Review 的地方，它记录着代码的具体改动，参与者具体的讨论过程。好的 MR 应该做到以下几点：

*  **单一**：一个 MR 应该只解决一个单一的问题，无论是修复一个 bug，还是实现一个新 feature。Author 应该避免一个 MR 包含不同意图的代码改动。单一的 MR 能帮助 Reviewer 快速地了解代码改动的动机，能有针对性地进行 Review。
 *  **短小**：MR 应该尽量地小，比如一个 feature 引入了较多的改动，需要考虑是否可以拆成独立的几块实现，分开提 MR，比如接口定义、接口实现、逻辑对接等拆分开。
 *  **详细**: 这里说的详细是指 MR 应该尽可能地详细描述它的背景和动机，可以是在 MR 的描述中详细体现，也可以是连接到具体 issue 或 tapd 单中。需要达到的目的是，其他人翻开一个 MR 能知道当时做这个改动的背景以及动机。

**2.1.2. Commit Message**

之前翻看了不少现存的项目代码，看到不少的 Commit Message 写得比较简单，例如一连串的 “update”, “fix”，从这些 Commit Message 中完全看不出做了什么改动，想想如果之后想要定位之前的某个改动，该从哪里下手。

目前 Commit Message 规范比较常见的有 **[Angular 团队的规范][Angular]**，并由此衍生出了 **[Conventional Commits Specification][]**，可以参照此 Specification 约定 Commit Message 格式规范。

大体分三行：

*  **【标题行】** 必填, 描述主要修改类型和内容。
 *  **【主题内容】** 描述为什么修改, 做了什么样的修改, 以及这么做的思路等等。
 *  **【页脚注释】** 放 Breaking Changes 或 Closed Issues

其中 type 是 Commit 的类型，可以有以下取值：

*  feat：新特性
 *  fix：修改 bug
 *  refactor：代码重构
 *  docs：文档更新
 *  style：代码格式修改
 *  test：测试用例修改
 *  chore：其他修改, 比如构建流程, 依赖管理

其中 scope 表示的是 Commit 影响的范围，比如 ui，utils，build 等，是一个可选内容。

其中 subject 是 Commit 的概述，body 是 Commit 的具体内容。

例如：

fix: correct minor typos in codesee the issue for details on typos fixed.Refs #133

Commit Message 可以在 git 中配置模板，这样可以在 vim 中展示出模板，另外可有工具帮助我们生成和约束 Commit Message，例如 commitizen/cz-cli，这里不再具体说明。

**2.1.3. CI 通过**

CI（Continuous Integration），持续集成可以帮助我们自动发现很多代码中的基本问题，在合适的静态代码检查（lint）配置和良好的单元测试覆盖下，CI 可以有效地提高代码的质量。很多人都低估了静态代码检查的能力，实际上现在常见语言的静态代码检查已经能帮助发现不少的 bug 和隐患。对于 Go 语言，可以配置 golangci-lint 来做代码检查，单元测试根据实际情况可以制定相应的标准，比如覆盖率 60%，其中关键的代码逻辑尽量全面覆盖。

**提交代码 Review 前需要确保 CI 执行通过**，这也是为了节省 Reviewer 的时间，能够通过自动化解决的事情，尽量不要让 Reviewer 来做，而 Reviewer 发现 CI 未过的 MR 也可以要求 Author 先解决 CI 问题。

**2.1.4. Self-Review**

我们一般代码 Review 都是找他人来进行 Review，其实负责任的 Author 在邀请他人来代码 Review 前也需要自己简单 Review 一遍，即 Self-Review。

Self-Review 的目的包括：

*  发现那些明显的疏忽，如代码 debug 过程中留下的不必要的痕迹，比如 fmt.Println(…)，不小心注释掉的代码。
 *  之前被 Reviewer 多次提出过的问题。
 *  Commits 是否正常，在多人协作的情况下 MR 中否带入了不相关的 Commit。
 *  Commit Message 是否合适。

Self-Review 是一个非常快速的过程，从我个人的经验，一般 1-2 分钟即可完成，所以推荐大家养成 Self-Review 的习惯。

### **2.2. 该找谁来 Review** ###

从目的出发，可以从以下几方面考虑 Reviewer：

*  提高代码质量。所以首先应该找和代码改动紧密相关的研发人员参与 Review，比如一起开发某个功能，某个项目，或者一起参与了方案设计讨论并给出了有价值意见的研发。
 *  获取意见。找有相关经验的资深研发帮忙 Review，比如 Java 语言资深的研发、写过相同或类似系统/功能的研发。
 *  形成共识。如果涉及到不同团队或模块间的接口改动，或其他会影响其他人的改动，可以邀请相关团队或模块的接口人参与 Review，以对改动形成共识。
 *  质量把关。对于重要的代码库，可能会执行比较严格的质量把控，如果设置了必须的 Reviewer，这些 Reviewer 也会参与进来。
 *  变动告知。很多情况下一个代码库可能只有一个人维护，如果做了些比较特殊的变动，其他人很难发现。因此在做一些重要的但是理解起来不那么直接的地方的时候，最好告知一下相关的研发，以便他们能大概知道发生了什么。

### **2.3. 都 Review 些什么** ###

经常会有 Reviewer 拿到 MR 不知道该 Review 些什么，其实无论你参与对应项目的深入如何，都可以对代码进行 Review，也鼓励不同人从不同的深度、角度去帮助 Review。**代码 Review 没有固定的形式，它更像是一门艺术，唯一的提高办法就是实际参与进去。**

Review 的时候可以从以下几个方面入手：

**1）简单的 Review**

在 CI 通过的情况下，最简单的 Review 方式可能只需要这样：

> Reviewer：在实际环境中都验证过了吗？  
> Author：当然验证过了  
> Reviewer：LGTM

这是一种提醒式的 Review。确认一句：**是否在环境中验证过了**，或者进一步把能想到的重要的验收点提出来确认一遍。即使是这种最简单的 Review 实际上也是有价值的，我们很难保证所有研发都会在提 MR 前实际在环境中验证自己所做的修改，也很难保证单元测试、e2e 测试能 Cover 住所有的情况，Reviewer 基本上也不可能都自己去环境上跑一遍。让 Author 去确认实际上就是提醒 Author 去确保改动至少是真实有效的，尤其是对一些已发布版本的 Bugfix，一定要提醒实际自测通过。

类似的提醒还包括：相关的文档（外部的）是否相应更新了、这个改动是否会有兼容性的问题、性能是否有影响。他们的本质就是提醒 Author 自己去思考他们可能遗漏的问题。

**2）常规的 Review**

代码 Review 一般都会从代码风格、变量命名、语法统一处入手，当然这些应该更多的借助于 CI 等自动化手段来保证，但是在相关流程还不是很完善的前提下还是有必要进行关注。

此外代码可读性、代码健壮性、代码可扩展性都是 Review 时关注的点。每一个关注的点都依赖于 Reviewer 的实际经验，这里只简单举几个例子：

**\{ 代码可读性 \}**

**代码是写给人看的**，因为没有不需要维护的代码，无论是 Author 自己后续维护代码，还是他人接手代码，能快速地理解代码逻辑是非常必要的。

代码 Review 的时候可以关注以下几点：

*  变量、方法的命名是否合适，是否真实反映了他们的目的，这方面网上可以找到不少的资料说明。
 *  实现的逻辑是否已有现成的库可以替代。如果有成熟的库可以使用，尽量不要自己去实现，因为可能会引入不必要的 bug。从我个人的角度，简洁（大白话就是代码少）是可读性一个很重要的指标。
 *  关于注释。代码注释不求多，而在于有效，以前也经历过代码注释要求至少达到 30% 以上的年代，现在看来过多依赖注释其实是对代码质量的不自信，好的代码应该尽量做到自解释。在实际过程中偶尔能看到代码逻辑实际已经清晰明了，但是用语句不怎么通的英文注释了一通，最后反而是看懂了代码才能理解注释到底说了啥。因此 Review 的时候，不必要的注释可以建议去掉。
 *  不好理解的地方要有恰当的注释。代码中如果有特殊处理、特殊的常量、或者不符合一般用法的逻辑需要特别注释说明一下。
 *  代码的组织。良好的代码应该有较好的封装以及层级，使得代码看起来清晰明了，比如 DAO 层、Service 层。

**\{ 代码健壮性 \}**

*  代码的改动是否会影响其他功能。
 *  用户参数是否做和细致的校验。
 *  有没有 Panic 的可能（针对 Go 的说法）。
 *  是否会破坏 API 的兼容性。

**\{ 可扩展性 \}**

当前的实现方式是否能兼容以后类似的扩展需求。比如在新增接口、API 或者调整参数以解决某一问题上，可以考虑是否后续会有其他类似情况发生。举几个例子：

*  假设我们需要定义一个 API 接口去获取一个用户的某些信息，例如联系方式等，我们定义的 API 就不能只返回这些信息，而是应该把用户相关的信息都返回。
 *  假设我要定义一个参数，虽然当前定义成单个元素即可满足，例如 string, int，但是以后是否会涉及到多个元素的场景，是否定义成 \[\]stirng, \[\]int 是更优的。

这里只是举了有限的一些例子，在实际 Review 过程中，Reviewer 可以根据自身的经验从各个角度提出优化的意见。一般需要重点看看：

*  你看不懂或疑惑的地方。
 *  打破你常规的地方。
 *  复杂的地方。

### **2.4. 如何进行** ###

Review 过程中鼓励 Reviewer 大胆 Comment，有不理解的地方，或者觉得不合适的地方都直接表达出来，**Author 对 MR 的 每个 Comment 也要做出反馈**，无论是展开讨论还是简单的给个 OK 都是有效的反馈。

Review 的过程可以是：

*  Author 在各项确认工作完成后，发起 Review，如果比较急，可以给重要的 Reviewer 发消息请求帮忙 Review。
 *  Reviewer 看到 MR 后应该先确认 MR 的背景和目的，如果不清楚也无法从 MR 中获取该信息，最好直接和 Author 沟通。
 *  Reviewer 直接在 MR 上提出自己的建议或者问题。
 *  Author 对每个 Comment 进行反馈，并展开必要的讨论。
 *  复杂的话题可以采用线下讨论以提高沟通效率。
 *  Author 处理完了所有的 Comment，也修改了代码后，需要在 MR 里 @ 一下相关 Reviewer 告知所有优化已经提交，如果时间比较急也可以直接和 Reviewer 沟通。
 *  Reviewer 确认没问题，给 MR 进行 Approve，一般简单的回复是 LGTM（Lood Good To Me），也可以对 Author 的工作进行赞赏，比如 “God Job” 等。
 *  Approve 后 MR 由谁来合并这个看自己选择。
 *  如果 Reviewer 提供了很多有用的建议帮助优化代码，Author 也可以礼貌性地感谢一下 Reviewer。

### **2.5. 关于 Comment** ###

代码 Review 很大程度上就是给代码挑毛病，而且一般预期挑的越多越好，但代码是 Author 写的，很多情况下或多或少会对 Author 造成不适。所以在 Comment 的时候需要尽量注意措辞，尤其是一些主观的东西，一般以建议的方式给出。当然对于原则性的问题，是要坚守质量标准的，甚至可以直接 Deny 掉 MR。

另外，参与者也**不要吝啬你的溢美之词**，无论是 Author 还是 Reviewer，在 Review 中发现了好的地方或好的建议，请给予对方以肯定和赞美，这样有助于 Review 有效的进行。

### **2.6. 参与者该抱着怎样的心态** ###

**2.6.1. Author**

首先需要明确一点，是 Author 自己对代码的质量负责，因此应当怀着感恩的心去看待坚持认真帮你 Review 代码的 Reviewer，因为并不是所有你加的 Reviewer 都有时间和精力来帮你提高代码质量。

也正式因为 Author 是自己代码的 owner，所以不要依赖于 Reviewer 去帮你守代码质量的大门，像 “代码 Review 的时候你怎么就没看出来”，“这不是你建议我这么做的吗” 这样的话千万别说。Reviewer 只是帮你提高代码质量，因此我们该做的工作都要去做，比如细致的 Reviewer 可能某些情况下直接提出了代码优化的建议，Comment 时贴上了优化的代码片段，Author 不能直接假设它一定是能正常工作的，而应该对它进行完整的验证。

对 Reviewer 给出的 Comment，不要有抵触的情绪，对你觉得不合理的建议，可以委婉地进行拒绝，或者详细说明自己的看法以及这么做的原因。有时候一个你觉得不合理的建议可能代表一个新的思考角度，也可能代表 Reviewer 自身代码能力上的不足，无论是哪个，无论最终是 Author 还是 Reviewer 得到了提高，都应该是好事。

**2.6.2. Reviewer**

Review 代码既是帮助提高代码质量的过程，也是 Reviewer 提高自己代码能力和沟通能力的过程。因此应该在 Review 的同时保持一个学习者的心态，既要发现对方代码中的缺陷，也要努力去发现代码中的亮点。**切记单纯以挑毛病的心态去 Review 代码。**

有不少 Reviewer 在写 Comment 的时候会犹豫，担心自己提出的问题或建议比较“蠢”，因此犹犹豫豫下看完了代码抱着一堆疑虑最终啥也没留下。其实在代码 Review 的时候大可不必有什么心里负担，有什么疑惑的、不清楚的地方或者有什么自己的想法，可以直接提出来，有时候一个简单的 Comment 就可能会激起 Author 和你的 Comment 毫不相干的新思路。再者你即使没帮 Author 提高代码，让 Author 帮你提高思考不也是建不错的事情吗。

Reviewer 也不需要去关注自己的“产出”，并不是一定要提出一堆问题才是好的代码 Review，Author 代码写得很棒也是很正常的，如果从你的角度觉得代码没问题，大胆给个 LGTM 甚至是 Approve。

**如果大家对java架构相关感兴趣，可以关注下面公众号，会持续更新java基础面试题, netty, spring boot,spring cloud等系列文章，一系列干货随时送达, 超神之路从此展开, BTAJ不再是梦想!**

![架构殿堂][20200415234301759.jpg]

[Angular]: https://link.zhihu.com/?target=https%3A//github.com/angular/angular.js/blob/master/DEVELOPERS.md%23-git-commit-guidelines
[Conventional Commits Specification]: https://link.zhihu.com/?target=https%3A//www.conventionalcommits.org/en/v1.0.0/
[20200415234301759.jpg]: /images/20221122/b40849e6d2f14501a457c0569374aba6.png