如何做好代码评审
“三人行，必有我师焉；择其善者而从之，其不善者而改之”
《论语》
前言
相信任何对技术有追求的工程师都希望提高自己的编码水平，任何团队都希望产品代码能具有统一风格，代码就像是出自同一人之手，降低认知负荷。
很多团队也都探索过 Code Review 实践，最终却不幸流于形式，虎头蛇尾，造成这种现象的原因是多方面的，团队文化、奖励机制、成本和收益不对等等问题：
- 缺乏 Code Review 文化，没有形成代码评审的统一认知，破窗效应
- 管理者对 Code Review 的关注度不够
- 在评审过程中不知道该关注什么，感受不到 Code Review 带来的收益
- 缺乏 Code Review 最佳实践，导致成本高，在摸索过程中逐渐失去耐心
- 产品业务赶工期，来不及做 Code Review
- 对 Code Review 工具熟悉程度不够
1. 为什么代码评审很重要
- 工程师成长的最佳场景：工程师之间通过 Code Review 对代码设计、实现、规范相关的讨论和交流，是相互学习和成长的最佳场景。软件工程能力只有从代码实践中学到，坚持高标准 Code Review，是团队工程能力提高的必要条件。
- 尽早发现 Bug 和设计中存在的问题：问题发现得越早，修复的代价越小， Code Review 就是把问题的发现尽量提前。
- 团队知识共享：代码入库之后，就从个人的代码变成了团队的代码， Code Review 可以帮助其他开发者了解这些代码的设计思想、实现方式，另外， Code Review 的讨论记录、标题、描述信息可以作为参考文档，帮助他人理解代码、追溯逻辑、查找问题。
- 针对某个特定方面提高质量：针对比较专业的领域，如安全、性能、UI 等，可以邀请专家进行专项审查，另外，复杂场景和高风险代码，也可以通过团队集思广益、查缺补漏的方式来降低上线风险。
- 统一编码风格：这也是 Code Review 的常见场景，但最好能通过工具，如 Style 插件/P3C 扫描等配置自动化任务来实现机器审查。进行 Code Review ，最好不要过度消耗在风格的检查上。
- 引导开发者编程心态：如果在编程过程中就知道一定会有同事将检查你的代码，那么你编程心态就会完全不同，会自然而然的更加注重代码质量。
2. 把 Code Review 作为一种制度
2.1 发布流水线上配置卡点
在发布流程中，设置 Code Review 卡点， CR 不通过不允许发布
2.2 代码入库前强制评审
如果代码仓库并不利用发布平台进行发布后入库，同时也希望能开发代码入库前的 Code Review 检查，可以在代码平台上开启开关来进行强制入库检查。之后可以通过在本地客户端发起代码评审，可以保证任何 Commit 在入库之前都经过了评审。这种方式可以像提交一个普通分支一样提交评审。
git push origin HEAD:refs/for/分支名 -o title="feat: add xxx config"
-o description="a description" -o reviewer=xxx
3. 形成 Code Review 文化
把 Code Review 作为上线发布流程或者代码入库的卡点并不代表 Code Review 这件事就可以执行的很好。如果仅仅只是当作一个流程制度，那么就可能会流于形式，最终结果就是看起来做了 Code Review ，但没有人认真审查，随便看下就通过了，或者发现问题也不愿意修改。
要想把 Code Review 这件事做好，必须让 Code Review 变成团队的一种文化，开发人员从心底接受这件事，并认真执行，否则如果只有部分同学认同容易形成破窗效应，久而久之，Code Review 就无法真正执行下去，下列这些方面可以参考：
- 开发人员认识到 Code Review 这件事为自己、为团队带来的好处，形成统一的共识比强制执行要事半功倍
- 充分发挥技术带头人和架构师的带头作用，做好表率，榜样的力量很重要
- 对于管理者/架构师/评审人来说，对认真完成 Code Review 的同学以及好的 Code Review 代码实现，不要吝啬自己的赞美，可以通过给 Code Review 点赞和评论等方式给予积极回应
- 把 Code Review 要作为开发任务的一部分，甚至面向 Code Review 编程，一旦面向 Code Review 编程，代码需要经过别人的审核才能入库，你自然而然在意代码质量
- 把 Code Review 当做团队核心的数据资产，也把 Code Review 当做技术人的名片
- 将 Code Review 同步给团队内更多的同学，代码平台支持邮件组通知功能，申请一个公共邮件组，将团队 Leader/架构师等角色组织起来，可以起到很好的监督督促作用
4. 提高评审效率和评审质量的实践
Code Review 的执行，很大部分程度上依赖于评审人的认真审查，以及被评审人的积极配合，两者缺一不可。但即使是团队已经初步形成了评审文化，但如果评审的成本过高，也会极大的打击评审人和被评审人的积极性，所以必须对 Code Review 进行行为规范来最大程度的降低 Code Review 时间成本，提升评审人和被评审人的沟通效率。这一章节会介绍不同角色对应的 Code Review 规范。
4.1 作为作者
4.1.1 确保 Code Review 只做一件事
保持单一职责原则可以有效避免 Code Review 成本过高。一个 Code Review 应该只负责一件事，处理了一个 Feature，解决了一个 Bug，新增一个组件或功能。
单一职责会让 Code Review 理解成本最小，不要为了省事在一个 Code Review 里掺杂太多其它修改，所以下列行为是不推荐的：
- 在处理需求时觉得的与本需求毫无关联的变量命名写的不好，顺手修改
- 在处理需求时觉得某个与本需求毫无关联的 Method 设计的不优雅，顺手重构
- 在处理需求时发现之前的代码格式不规范，顺手格式化整个文件
这些行为都是在增加评审成本，应该新起一个独立的 Code Review 来完成，而不是在本次 Code Review 中做。Code Review 的规模和范围并不以代码修改规模来作为唯一评判标准，也要判断该 Code Review 是不是只在做一件事。另外，如果 Code Review 是为了实现代码清理和代码格式化等优化，也不要夹带私货，避免引入 Bugfix 和 Feature。
4.1.2 发起 Code Review 的时机：尽量提前，开发中小步快跑
避免发起临门一脚的 Code Review，临门一脚指的是快接近发布上线时间才发起的 Code Review，开发代码都是为了发布上线，有一些需求会有发布上线的时间点限制，但发起临门一脚的 Code Review 会显著的将 Code Review 推向流于形式的方向。根据经验，Code Review 越左移，修改代码的成本越低，修改意愿也越高，什么是左移？先了解一下常规的发布上线流水线流程，有两种发起 Code Review 的时机，从 CR 实践落地效果上看，第一种 CR 发起的时机更好
-
时机 1：和 CI 并行的 Code Review
-
时机 2：在上线发布之前发起 Code Review
从流水线上来看，许多人会在临近上线时，在靠右的地方快合并主分支的时候才进行 Code Review ，这个时候修改成本就很高，因为代码已经自测/测试通过，如果因为 Code Review 有问题需要重新修改代码，功能本身又要自测或者回归测试，占用更多的时间。而且因为已经临近上线，却因为 Code Review 被打回，开发人员愿意重构代码的意愿也会很低，明明发现问题，又因为上线压力，不打回不符合规范的代码，久而久之大家会失去对 Code Review 的敬畏心理， Code Review 也会慢慢流于形式变成应用发布的过场之一，既无法提高代码质量，降低系统 Bug，也不能提升开发人员的水平，反而降低的开发团队的效率，所以我们要尽量避免在上线之前才发起评审，最好的评审发起时机应该是在开发完毕后立即提交代码评审。但也有一些复杂场景需要反复进行测试和验证，对于这类 Case 也可以在 CI 流程完毕后再发起。
4.1.3 尽可能提前做好代码自审
在提交Code Review之前可以借助工具进行一次查缺补漏，避免将低级错误和不完善的代码展现给评审人。例如借助代码平台的新建评审页面展示提交内容，查缺补漏。
4.1.4 编写语义明确的标题
不论是 Code Review详情页，还是列表页面，标题都是评审者最先接触到的信息。
如下图，从评审列表无法判断 Code Review 所完成的事项，是否是在处理 Feature 还是 Bug，也无法判断紧急程度和优先级。
推荐使用如下格式来规范标题：
- 修复线上 bug：fix(模块)：标题
- 修复线上紧急 bug：紧急 fix(模块)：标题
- 上线新的 feature：feature(模块)：标题也有团队使用 feat 来表示 feature
模块可以是每个平台约定俗成的模块，比如代码平台的模块包括：Code Search、 Code Review 、Issues、Wiki 等，从评审列表就可以获知大部分信息，当前 Code Review 在解决什么模块的什么问题，也可以根据前缀大概判断紧急程度。规范之后从列表对 Code Review 的意图一目了然，Code Review 也是团队的重要数据资产，通过规范的标题可以对模块的发展脉络进行方便检索。
4.1.5 编写详细的描述
Code Review 的描述也是最重要的信息输入之一，是 Code Review 发起人和代码评审人之间的对话，发起人有义务将自己的编码逻辑、修改范围、可能影响介绍给评审人。评审人在阅读代码之前就了解你的背景和思路，比阅读代码过程中再逐步领悟效率要高得多。评审人也有权利拒绝评审无任何描述信息的 Code Review。
- 详细的描述信息能大大降低评审人对你的代码编写逻辑理解成本
- 在编辑描述信息的同时也可以对自己的代码逻辑做进一步的梳理，查缺补漏
- 描述信息也是在代码发布上线后追溯代码修改逻辑的一种重要手段
描述部分建议提供以下信息：
背景：本次修改的原因是什么
思路和改动点：大概的思路是什么，改动了哪些方面可能造成的影响介绍一下
4.1.6 开发过程中的Code Review可以使用 WIP 前缀来标注
WIP：work in progress
评审人在注意到 WIP 开头的 Code Review 时可以暂时忽略
我们并不能保证每次评审发起后都是一个完整功能或者 fix，例如在发起评审后突然意识到有设计缺陷，或者意识到提交的代码缺少了 UT 覆盖等问题，评审人在不知情的情况下可能随时对你的代码进行评审，为了避免评审人将时间浪费在还未完成开发的 Code Review 上，创建人有义务将标题改成 WIP 前缀，Code 平台会限制这种 Code Review 的合并行为，评审人可以忽略这种评审。
4.1.7 解决所有 CheckList
每个评审创建人都要对自己的 Code Review 负责，确保 Code Review 达到最终可合并状态，这里的可合并状态并不是有人通过即可，还包括测试用例的状态，Trybots 的状态，评论的状态。 Code Review 会在突出位置展示当前未完成的任务。
作者需要确保 Code Review 的 Checklist 全部解决，其中包括：
- 确保评审人通过评审
- 解决评论全部解决
- 确保测试卡点任务全部通过
4.1.8 尊重评审人的评论
尊重评审人的劳动，针对评审人提出建议或者反馈的评论，请给出适当的回应，虽然 Code Review 平台允许创建人将评论置为已解决，但不进行任何回复是不恰当的。
保持谦逊，任何人的代码都可能有改进的空间，哪怕你是这个领域的专家，提交的代码都有可能出现错误，承认不完美才是专业的表现。
4.1.9 利用格式化扫描插件来实现风格统一
尽管希望借助Code Review来实现代码仓库的风格统一，但将宝贵的精力用于敦促代码风格统一上是得不偿失的，各团队技术负责人配置自动化扫描插件来完成，例如Eslint、P3C、SpotLess。并配置Aone实验室任务、配置CR卡点等。
针对 Java 语法就有一些常见的风格扫描插件，例如：
- 谷歌的 SpotLess ，SpotLess——让你的代码一尘不染，这个插件不仅可以扫描代码风格，还能帮助格式化代码
- P3C 代码规约，P3C Java 规约扫描 在 CR 时可以进行扫描格式化
4.2 作为评审人的最佳实践
4.2.1 尽快评审
把评审代码作为工作的一部分，和开发者尽快发起评审的原则一样，评审人需要留出足够的时间尽快完成评审。越早发现问题，修改代码的成本越低，修改意愿也越高。如果当前没有时间评审，也要明确告诉评审人会在什么时间点前完成，甚至将评审工作转交给其它同事完成。
4.2.2 对逻辑和质量进行评审
4.2.2.1 实现逻辑
Code Review 应该着重对业务逻辑进行检查
- 逻辑问题：是否有未考虑到全局的设计或现有的某些业务细节，边界条件是否考虑充分等
- 安全问题：是否存在安全隐患，例如是否存在 SQL 注入、CSRF、越权等安全漏洞
- 性能问题：是否会产生性能上的损害，例如慢 SQL、热点 key、数据倾斜等性能问题
- 线上风险：是否会对稳定性产生潜在影响，例如是否产生死锁、死循环、FullGC、OOM 等
- 未知影响：是否对体验产生潜在影响，例如某 Feature 的上线会潜在影响了其它功能的表现等
- 测试用例：是否有有效的单元测试用例验证逻辑，测试用例的分支覆盖度和代码行覆盖等
4.2.2.2 代码质量
提升代码质量是Code Review的重要目的，Code Review 可以对代码风格、规范、可维护性等进行检查。
- 编码规范： Code Review 的目的之一是协同，是希望整个团队的代码编写能像出自一人之手
- 可读性： Code Review 是一个很好的测验代码可读性的手段，避免使用奇淫巧技，避免过度拆分
- 可维护性：代码的可维护性是由很多因素协同作用的结果，代码的可读性好、简洁、可扩展性好，就会使得代码易维护
- 重复度：遵守 Don’t Repeat Yourself 原则，尽量减少重复代码的编写，复用已有的代码
- 可测性：代码可测试性的好坏，同样可以反应代码质量的好坏，代码的可测试性差难以编写单元测试用例
但这部分审查不应该作为评审重点，产品或者技术负责人应当选择合适的扫描插件并配置合适的扫描任务来完成扫描和准入。
4.2.3 针对代码，而不是作者
Code Review 针对的是代码而非作者，尽量不要指责代码作者的行为和态度，同时也需要能够接受不同的方案，任何问题总有很多不同的方案，你有的只是你个人倾向的方案，但别人的方案可能同样没有问题。
- 对于需要共同遵守的开发规范和一些设计问题，比如代码规约等一定要指出来给予改进
- 对于最佳实践，可以视情况而定，如果是团队已经认可的统一的最佳实践需要明确指出推动改进
- 对于个人风格偏好，可以适当做出一些妥协，比如有的同学习惯把函数拆分的非常小，有的同学就没有这样的习惯，只要不是影响代码理解，这都只是个人习惯，不要做过多争论
4.2.4 避免过度追求完美
避免苛求每一行的代码都是完美的，避免以找问题的心态进行 Code Review，要抓住 Code Review 的关注重点反馈，另外，并非一定要找到问题才证明你在认真评审，若针对作者的每一行代码评审，很可能会惹恼到作者，也影响到他们对你的反馈的接受。
4.2.5 不要吝啬自己的赞美
毫不吝啬的正向反馈也有助于 Code Review 的实践落地，在日常 CR 中，通常的反馈都是问题或建议，鲜有称赞，但代码评审的目的不应该仅仅是发现错误和问题，还应该鼓励和指导开发人员所完成的出色工作。可以通过分享、点赞等方式等来评价。
4.2.6 明确评论是否要解决
评审人在发起评论时需要明确指出该评论是否需要被解决，代码平台支持评论标签，支持标注评论是否需要被解决，被标注为不需要解决的纯讨论性评论不会被计入 Check List。对于代码修改建议，需要给出明确的论据或者示例。
另外可以利用标题的描述对 Review 的评论进行分级，告诉开发者应该如何处理这个评论，例如给评论增加Optional前缀。
- [optional]：在评论前面加上一个 [optional] 标记，表示这个代码行的问题可改可不改
- [optional]：对这个逻辑太复杂的函数进行拆分
- [optional]：函数的参数太多，建议进行对参数进行封装避免出错
4.2.7 避免使用反问句来评价
避免使用反问句来质疑代码逻辑，通常来说当你 Code Review 代码时保持礼貌和尊重能使开发人员更加清晰，得到更多帮助。也要避免用负面词汇来评价代码。
🙅🏻♀️ 不好的例子: “Catch 异常都不精确的？直接一把抓的？那怎么区分出不同的异常呢？”
👌 好的例子: “我并没有发现这个并发模块给程序带来了多少帮助，并且还增加了程序的复杂性，因此我认为这段代码最好是用单线程而不是多线程。
5. 小结
想要推动 Code Review 在团队中更好地实施，很重要的一点便是增强团队成员对 Code Review 的认同感。 如果团队成员不认可 Code Review，即使强制设置了 Code Review 流程，也是形同虚设，反而影响正常开发流程的效率。相反的，如果团队同学都认可“ Code Review 是提升代码质量、提升研发效率的核心手段”，那么团队成员的主观能动性会越来越高，互相学习的氛围越来越浓，帮助别人的同时，也能有效提升自己的代码能力。Code Review 的价值得以最大化，最终实现整体提升研发效率提升。