如何做好 Code Review
作为技术工作者，你会发现我们的工作中有很多看似“玄学"的部分。比如性能优化，比如技术管理，比如这篇文章要说的 Code Review。对于这些技术领域，似乎每个人都有自己的一些经验，但真正能把这些事情做好的公司、团队、个人据我了解又比较稀少。以 Code Review 为例，国内即便是一些大厂，并没有把这件事情做好。我经常会在面试中问一些在大厂工作多年的候选人“你们团队是如何做好 Code Review 的？”。从回答中能发现很少有人对此有较深入思考，也侧面反映出能把这件事做好的团队并不多。
为什么会这样？按我目前理解，解决好这些复杂问题，仅仅凭借一些离散的点的经验并不足够，我们还需要借助一些思考框架，以及基于思考框架具象出的标准操作流程。
Google 作为最先进的互联网公司之一，积累了大量优秀的工程实践经验。关于如何做好 Code Review，也有自己的方法论：Code Review Developer Guide，我周末花了两个多小时仔细阅读了一遍，非常受启发。下面的内容是我基于这个 Code Review 指南（以下简称“指南"），结合我自己的思考，以 Code Review 工作流的方式来梳理“如何做好 Code Review"。如果你有时间，强烈建议去阅读一下 Google 的原文。
零、 核心原则
指南的第一篇 The Standard of Code Review，讲解了 Code Review 的核心原则。总结一句话：作为评审者(以下用英文 Reviewer 指代)，接受本次代码改动的标准不是改动的代码必须完美，而是这次改动能提高整个代码库的健康度。怎么理解这个核心原则呢？首先并不存在“完美代码"，没有是谁能定义完美代码。我们需要的是更好的代码，追求的是代码库的持续改进。“完美代码”是静态思维，“持续改进”是动态思维。Reviewer 不应该以“完美代码"为由拒绝能够改进整个系统可读、可维护性的代码改动。
Code Review 的目的是为了项目在长期维护中能持续快速地迭代，高效高质量地支撑业务交付。如果为了追求所谓“完美"，而导致项目延误，就有点本末倒置了。当然这并不是说要降低标准，基本的准入门槛（比如统一的风格指南，Sonar 质量域等）要严格遵守，在此基础上，如果能改善代码库（至少不会腐化代码库），那就不应该求全责备追求完美。
这是 Code Review 的第一性原则。不过在通篇读完这个指南后，我发现其实还有一个重要原则，那就是无论 Reviewer 还是提交改动的开发（以下以英文 Developer 指代），都需要有同理心。因为 Code Review 是双方的协作行为，如何高效愉快地把这件事做好，有赖于双方都能多替对方想一想，具体应该怎么做下面会详细展开讲。
小结，Code Review 的核心原则：
- 持续改进原则：接受改动的标准是本次提交的代码是否能改善（至少不腐化）代码库，要追求持续改进，不追求完美代码。
- 同理心原则：Code Review 是涉及 Reviewer 和 Developer 的协作行为，要做好这件事，双方都应该多替对方想一想。
壹、Developer 如何提交改动
很多公司都会用 gitlab 作为代码版本管理和协作的工具。Developer 发起 Code Review 即是提交一个 MR（Merge Request），并且把这个 MR 指派给对应的 Reviewer。那 Developer 究竟应该如何提交这次改动来提高 Code Review 的效率呢？
指南的 The CL author’s guide 部分讲解了两个最重要的操作建议：
- 写好这次改动的描述
- 每次提交小的改动
如何理解这两个建议？
关于“写好这次改动的描述”：首先要理解为什么。Reviewer 通常是代码库（或其中某个模块）的负责人，也通常是更资深的开发，这也意味着他们大概率很忙，时间很宝贵。当然即便 Reviewer 是普通开发，出于尊重别人时间的原则（同理心，你也希望别人尊重你的时间），作为发起这次 Code Review 的 Developer，就有责任把改动描述清楚：改动了什么，为什么这么改？除此之外，写好改动描述还有一个必要性：未来的开发者有可能需要弄明白你这次改动的原因，如果能追溯到你提交的这次改动，并看到清晰的改动描述，会比通过阅读代码理解背后原因高效很多。
那怎么写好改动的描述？Google 建议的格式如下：
第一行：用完整的句子说清楚改动了什么，并跟随一个空行
内容：详细描述这次改动的内容，提供必要的理解这些改动的补充信息，比如究竟解决什么问题，为什么这样改，这样改可能有什么问题。如果可以的话，补充其他的一些背景信息，比如相关的设计文档、bug 修复任务连接等。
举一个好的例子：
引入 prettier 来保证代码格式一致。
之前的项目没有好的代码格式约束工具，不同人写的代码格式相差太大，比如有人喜欢一行很长，有人喜欢很短，有人写代码没有空行，有人喜欢随意加空行。通过引入目前前端最受欢迎的代码自动格式化工具 prettier 来统一项目的代码格式规范。
关于“每次提交小的改动"：首先也需要理解为什么。每次提交小的改动有一些显而易见的好处，比如 review 的过程会更快也更全面，更不容易引入 bug，如果代码设计有问题也能尽早被发现，而不是写了一大推被驳回重写浪费时间等等。另外，也是基于“同理心"的原则，假如一次提交大量的改动，对于 Reviewer 是巨大的负担。他需要在繁重任务之外，抽出一整块大的时间帮你 review 代码。如果他尽责，他要付出很大的努力尽量避免你的这次改动引入 bug，会和你来回讨论多次，这个过程很漫长，可能也会显著影响项目的进度。当然也可能，他会选择性地 review，这可能导致代码库腐化。无论哪种情况，都和 Code Review 的最终目标背道而驰。所以作为提交改动的 Developer 有责任每次提交小的改动。
那什么是小的改动？“小"的标准，不仅仅是代码改动行数，而是应该以对 Reviewer 心智负担的程度来衡量。当然，通常建议改动在 100 行左右为宜，当然你提交 200 行也是可以的，但如果你提交 500 行，甚至 1000 行，那肯定会给 Reviewer 造成大的负担。在提交改动之前你应该问问自己，如果我作为 Reviewer，我能比较容易评审这次改动吗？如果答案是否定的，那就尽量把改动拆成更小的 MR 任务。这也是同理心原则的应用。
那又如何尽可能提交小的改动呢？Google 有一些建议，比如把代码重构、bug 修复、功能新增等不同改动目的的代码分开提交，当然最终的标尺在开发者自己心里，究竟什么代码是重构，什么是功能新增。如何拆分每个人也许也有自己的一些技巧和心得，但不管怎样，作为提交改动的 Developer 一定要把提交小的改动作为准则来执行，确保 Reviewer 在执行评审的时候不会太困难。
当然，也许真的有一些特殊情况，没办法拆分成小的改动，建议提前和 Reviewer 打招呼，让他们对需要投入多少时间精力有充足的准备。你看这也是同理心原则的应用。
贰、Reviewer 如何评审改动
在 Developer 按照以上的标准提交改动后，收到改动评审请求的 Reviewer 该怎么做呢？指南在How to do a code review 中提供了操作建议，我总结如下：
- 要尽可能快地响应 Review 请求
- 要多个维度地评审代码
- 要写好 Code Review 评论
下面分别对这三条操作建议进行解释。
关于“要尽可能快地响应 Review 请求”：Code Review 是研发中的一个环节，和其他流程节点一样，都是为了更好更快地做出产品或交付项目。如果 Code Review 太慢，会严重影响团队的研发效能。很多被 review 的开发也会因此对 Code Review 这件事本身产生反感，会觉得这是在阻碍自己，而不是帮助自己。
那究竟应该多快响应呢？Google 的建议是最好不要超过一个工作日。当然这里面可能会有一些矛盾。我们强调不应该打断程序员的心流，因为一旦被干扰，重新回到这种高效的工作状态要额外花费更多的时间。如果 Reviewer 正处在这种状态中，可以先忙完自己手上的工作，或者在会议后、午休后来做 Code Review。我认为快速响应的标准是，我们追求最大化团队的整体效率，不是 Reviewer 的效率，也不是提交改动者的效率。
这里需要强调的是，我们一直在说“快速响应”，而不是快速完成 Code Review，Code Review 本身当然也应该尽可能地快，但 Review 本身是需要花时间的，我们不能为了快而快，快的基础是提交的代码必须符合我们的标准。不过出于同理心原则，我们应该尽可能快地响应，减少提交改动的 Developer 的等待焦虑。如果 Reviwer 真的太忙没有时间，可以建议其他有资格的 Reviewer 来快速响应这次 Code Review，或者先提交一些对改动的整体建议。
前面我们提到作为 Developer 应该提交尽可能小的改动，但有可能有些人没有遵守这个准则。如果提交的改动很大，作为 Reviewer 可以要求 Developer 改成多个更小的 Code Review 请求。假如真的没办法拆分，并且你没有大量的时间快速地做完整评审，你至少应该提交一些反馈，比如对整体设计的评论，好让提交改动的 Developer 能够快速地采取改进行动。总体原则是，在不牺牲代码健康度的情况下，尽可能不阻塞 Developer 的工作流。
只要坚持这样做，就能形成良好的正向循环，随着时间推移，Code Review 本身需要花费的时间也会越来越少，速度会越来越快。
关于“要多个维度地评审代码”：这说的是作为 Reviewer，究竟应该如何评审代码，究竟应该关注代码质量的哪些维度？也许不同的技术栈会有一些不同，这部分内容也是很多 Code Review 经验分享文章着墨的地方。这里不展开细说，因为细节会非常多。以下是 Google 建议的几个维度：
- 整体设计：代码应该有好的设计。作为 Reviewer 应该多问几个问题：改动涉及的这些代码真的有用吗？这个改动应该放在项目中还是依赖库中？我们现在需要添加这个功能吗？如果整体设计有问题，应该尽早反馈给 Developer，避免他在错误的方向上浪费太多时间。
- 功能影响：确保这次改动对代码的用户（终端用户和其他开发者）是友好的。也可以问自己一些问题：这次改动会带来不好的用户体验（性能稳定性）吗？UI 没有被破坏吧？对其他并行开发的部分不会有影响吧？
- 复杂性：代码不应该比它需要的更复杂。如果一段代码不能很好地被理解，那么代码很可能就过于复杂了。复杂的代码在后续的维护中也更容易引入 bug。另外，不要过度工程，鼓励 Devloper 解决当下需要解决的问题，而不是未来可能要解决的问题。
- 单元测试：代码应该要有合理的单元测试。这应该是所有开发的常识。并且单测本身要有很好的设计，测试代码要避免过于复杂。
- 命名：应该要有足够好的命名，代码即文档。好的命名应该包含足够的信息，不需要额外的注释。
- 注释：注释应该解释为什么，不应该解释代码在做什么。如果代码本身不能解释自己，代码应该重构得更简单。
- 文档：如果代码变动，应该思考相关的文档是否也应该变动，应该保证文档及时更新。
- 风格指南：代码必须要遵守公共的风格指南，保持风格一致性。
除此之外，还有一些建议：
- 看每一行代码：原则上，作为 Reviewer 应该仔细地看每一行代码。当然这里面还是有一些尺度需要自己把握，比如一些配置类文件、数据文件等，可能只需要扫过一眼就可以。而核心的代码可能需要花费更多的时间，而不是只看一遍。
- 关注改动的上下文。有时候改动只有几行代码。如果仅仅看这几行代码可能发现不了问题，这个时候应该看看改动代码的上下文，很可能了解上下文之后会发现潜在的问题。永远记住最上面的原则，提交的代码不能降低代码库的健康度。
- 不要吝惜赞美。如果你看到 Developer 提交的改动有一些好的地方，比如改善了原有的方法，比如你从中学到了一些新技巧，你应该在评论中赞美他。
最后，关于“要写好 Code Review 评论”：原因是显而易见的，这是 Code Review 形成闭环的关键步骤，给 Developer 提供有效的反馈。指南也帮我们总结了几条操作建议：
- 要友好。要对事不对人，这样能避免不必要的争论。这其实也是同理心原则的应用，被人不礼貌地数落多少会有点难受。不好的句式：“你不应该这样写，xxx”，好的句式：“这个地方用这种方式可能存在 xxx 的问题，如果改成 xxx 是否会更好呢？”
- 解释为什么。解释为什么是影响力原则里的重要武器。虽然不要求每一条评论都带上解释，但如果你能说明你为什么这样评论，把理由说清楚，被评审者可能会更容易接受，也更能心平气和讨论。
- 给出指导。这个需要自己权衡，有时候指出问题就可以，因为 Developer 对改动代码的背景和上下文更清楚，他们知道问题后或许能给出更好的解决方案。当然有时候，作为 Reviewer 也可以提出自己的具体改进建议。
- 改进落到代码中。有时候你看不懂一段代码，需要 Developer 解释，但最好不要止于解释。应该让 Developer 将代码重构得更容易懂，或者在代码中添加必要的注释。这样后续的维护者能够更容易理解这部分代码。
叁、如何解决评审异见
这一部分主要是同理心原则的应用。在 Code Review 的过程中，对于某些代码，Developer 和 Reviewer 可能会有不同的意见，这很合理，因为不同人的视角、掌握的上下文不一样，也鼓励大家讨论。但我们应该尽量避免冲突。那如何避免冲突，冲突真的发生又该如何解决？对不同的角色，也有不同的操作建议。
对 Developer：
要记住 Code Review 的目标是提升代码库和产品的质量。 Reviewer 提的建议、批评都不是针对你个人的，而是在试图帮助你，帮助代码库，乃至整个公司。作为 Developer 应该尽可能地思考，这条评论试图提供给我什么建设性的信息？
无论怎样，不要带着情绪回应 review 评论。这只会引发不必要的冲突，如果真的很愤怒，可以先离开电脑屏幕出去透透气，等平静后再回来礼貌地回复评论。如果真的有 Reviewer 有一些不礼貌的评论，甚至人身攻击，那么可以找他当面沟通，或者通过邮件等方式，还是无效的话，可以将问题上升到你的主管。
对 Reviewer：
不要人身攻击。有时候 Reviewer 也不是真的人身攻击，而只是措辞不当。关于如何写好评论前面已经有论述，总之在写评论，以及和 Developer 讨论的时候，一定要基于技术事实且有礼貌地进行沟通，避免激发对方不愉快的情绪。对于谁是正确的，不要因为自己是 Reviewer 就自负，Developer 因为是代码改动者，对上下文信息的掌握更清楚，也许他的写法才是更好的。当然这不是说 Reviewer 要妥协，持续改进代码库的核心原则是底线，不能破，除非遇到紧急情况（最后一部分说明什么是紧急情况）。
最后一点对于 Reviewer 的建议是，如果你们讨论得到了结论，代码应该进一步优化，那么同样地，除非紧急情况，不能让优化延后改，因为一旦延后几乎就是永远不会改了。
如果双方在友好讨论沟通后，还是没有定论，那么可以邀请更多的相关开发参与讨论，或者拉更高层级的开发入局，让他来决定哪种做法更合理，更应该被接受。
肆、什么是紧急情况
很多事情都有例外。以上说的 Code Review 原则和操作建议对日常工作中绝大部分场景都适用，但在遇到紧急情况的时候可能需要做一些变通。不过我们需要对紧急情况有清晰定义，否则有可能会对不紧急的”紧急的情况“妥协代码的健康度。
什么是紧急情况？
- 线上出现生产事故（P0、P1 等），显著影响了用户的正常使用。
- 线上发现重大安全隐患，需要紧急修复漏洞。
- 一个小的修改来避免大版本发布后的回退。
- 有对合作方承诺了硬性的截止日期。
- ...
这个清单可以继续增加，但需要记住一个总体原则：如果这件事不尽快处理会出现灾难性的后果。所以，以下的这些看似紧急的事情其实并不属于紧急情况：
- 产品经理说要在某天之前上线（但这一天并没有特殊理由，不是硬性截止日期）。
- Developer 已经在这个功能上花了很多时间，想要尽快合并代码。
- 今天是周四，是最后一个发布日，Developer 想在这一天发布。
- ...
这个清单可以列得更长。但它们不会带来灾难性后果，所以不能因为这些“紧急”情况而妥协代码质量。
以上是基于 Google 工程实践总结出的关于 Code Review 的原则、方法以及背后洞见，如果你有什么想法欢迎留言交流。