开发团队应该如何高效进行Code Review

当前团队内部在每个需求开发完成，进入测试之前，会组织会议进行Code Review，主要是针对代码一行一行讲解，但是最终的效果并不是很好，尤其是很多时候对于具体技术方案都不清楚的情况下，直接讲解代码，非常低效。

此外，按照开发的思路对代码一行行讲，对于开展 CR 的价值并不大，参与评审同学只能针对格式，代码规范等进行 review，对于更顶层的代码模式设计和实现技术思想，基本上都没有体现出来，也因此无法发现一些隐层的实现细节上的漏洞。

最后，会议上的code review，时间有限，从这么多次的体验下来，没办法把所有问题反馈完整，并且后期的改造也没有完整版的 review 评论参考，无法形成一个闭环。

因此，本文就对Code Review 进行一些思考，以及通过对外部其他同行的落地实践经验进行调研梳理后，期待形成一个团队内部可推行的Code Review方案。

一、Code Review 价值和目标

首先，我们需要明确和对齐Code Review价值和目标，如果这个得不到认可，那么就不需要在团队内部推行Code Review了。例如，有些团队认为，在创新业务快速发展，不断试错的情况下，CR并不是有价值的，让业务活下来才是重中之重，代码的随意和设计的扩展性差，无所谓；等到业务稳定了再进行重构和重视代码质量。那么，在内部推行Code Review 价值就不大了，至于这些团队的想法，无可厚非，不要过于纠结对不对，其实并不重要。

接下来，从自己的角度来说说Code Review价值，同时，也是自己对新CR模式落地后能达到的期望目标：

• 发现问题，提升代码质量。
• 交流技术，提升团队氛围。
• 维护约定，提升规范意识。
• 共享业务，提升团队业务感。

1.1 发现问题，提升代码质量

通过Code Review 找到代码中的bug，是最基础也是最容易想到的CR价值，也是很多团队推Code Review的说辞。但是，这反而不是我们进行Code Review的原因。

• 在进行代码review的时候，可能会发现一些代码处理不正确的bug，比如边界没有处理，可能存在NPE或者索引越界问题，或者对于一些条件判断的遗漏，比如说数据幂等情况下的异常处理，不需要重试，比如参数合法性验证缺少，导致后续的业务逻辑无法进行，等等。

• 此外，在进行代码review的时候，我们还可能发现特定异常exception没有被catch，打开的资源没有被close，线程池构造不合理，threadlocal 对象没有被remove操作，等等。这些问题，都很重要，但是不影响功能测试的正确性，因此测试的时候，可能并不能被发现。一些时候我们可以通过review找到这些代码bug。

但是，这些问题，其实很多时候，都能通过单元测试来排除出bug，或者通过一些静态代码扫描工具，或者代码规范工具，来完成问题代码提醒功能。例如，静态扫描工具sonarQube，idea 插件alibaba java coding guidelines 等。

也就是说，虽然，发现代码问题或者bug，确实是code review过程中经常被提到的，但是其实在code review之前，这些通过工具可以发现的问题和bug，就不应该在code review中被其他人发现到。

但是，我们期待通过Code Review可以发现一些隐藏比较深的bug，尤其是并发问题、分布式问题，或者找到一些可以做性能优化的点。不过，这对于参加review的同学技术和经验要求是比较高的。这里，主要侧重代码实现上的问题，一些方案设计上的问题，则应该在详细设计方案review审批上，被发现和提出来讨论。

• 线程安全。举个最常见的例子，比如多线程环境下使用global++来计数，这种可能通过工具和单测不容易发现的case。
• 分布式问题。例如，分布式环境下，基于网络之间的调用，除了明确的成功和失败之后，还会存在未知错误，这个错误在一些特定场景下，可能需要特别处理。比如支付场景下，将未知错误当做扣款失败来处理，就可能会导致多次扣款。
• 并发请求问题。这种还蛮多见的，大部分详细设计不会涉及到，所以只能在代码review的时候，去发现。
• 性能优化的点。比如缓存优化，缓存常用使用方式：查缓存，没有查数据库，写缓存。但是对于一些业务缓存命中低的特性，如果继续这样使用，显然是有问题的，比如黑白名单服务。

以上等等，就是在方案设计的时候，不会涉及或者蜻蜓点水而已；但是，代码实现的时候，需要重点关注的点，而这些点通过简单的测试和工具扫描是无法发现的。

有些人会觉得Code Review的代码bug全部可以通过单元测试/集成测试发现，这种要么就是测试用例写的特别完善，包括各种复杂的多线程，并发场景下的测试case；要么就是业务比较简单，或者只是在很稳定的系统上，一些简单的代码调整，不存在复杂的处理逻辑。

目前团队的测试全面性，还显然达不到。

1.2 维护约定，提升规范意识

在进行Code Review之前，团队内部应该有一个代码规范，或着大家一致认同的代码约定。那么，在进行Code Review的时候，当新代码挑战了之前的约定，那么就需要进行调整。要么去优化原来的规范约定，要么就需要开发者去修改代码。

按照约定的代码规范，保证新的CR代码风格一致，是进行Code Review一个大的基础性的价值。在团队最初就确定好了代码结构规范和命名规范等，这些约定好的规范就需要团队成员的每一次代码提交严格执行。否则，规范就会被束之高阁，代码五花八门，后期维护就是个灾难。

1.3 交流技术，提升团队氛围

一个良好的Code Review，可以提高整个团队的技术交流氛围。程序员经常会说：talk is cheap，show me code。因此，在对代码进行review的时候，其实是最佳的代码交流机会。

对于一个功能的实现，可能有非常多的方式，可以使用各种适合的设计模式，可以利用各种开源的辅助工具包，在review过程中，发现非常优秀的代码实现方式，或者提供更好的实现建议，在review中进行交流，甚至是激烈讨论，都可以让整个团队对于技术氛围的提升非常有帮助。

前提是，良好的Code Review机制。一个Code Review的成败，最主要是取决于代码审查者的态度、责任感和高质量要求的底线。

1.4 共享业务，提升团队业务感

很多团队在执行Code Review的时候，并不会认为提升团队业务知识，是通过CR可以做到的。这主要原因是，这些团队在Code Review的时候，更多关注于代码实现是否有bug，设计模式是否试用等。

开发同学了解业务，除了使用对应业务产品功能外，另外一个方式就是看代码。这说明，代码里面，是存在业务的。参与Code Review评审的同学，可以通过CR申请附带的文档和实现代码，去了解业务需求背景，业务本次优化调整，以及技术实现。

这样，需求开发同学就可以通过本次Code Review将业务知识传播出去，在整个团队内部进行共享。

1.5 最终目标-业务和技术能力的成长

归根结底，Code Review 的最终目的，还是为了团队成长 ^_^。

通过更高的代码质量和更多的业务知识分享，可以让业务系统有序健康可扩展，更快更好支持业务需求上线，同时团队成员都可以参与到业务本身的发展和迭代创新上。此外，技术上的交流和规范的优化升级，也可以让开发同学的技能得到提升。

二、Code Review 对象和时机

对Code Review价值认可之后，就可以考虑推行Code Review机制了。但是，任何一个商业组织都需要考虑投入产出比，也就是所谓的ROI。什么代码需要被Code Review，什么人建议参加Code Review，Code Review的形式，时机怎么选择，都是需要考虑的。
当然，如果你对成本无所谓，那可以直接落地Review了。

本质来讲，代码review是一件事，做事之前对齐价值和目标之后，就可以开始执行了。现在，给我们的就是，什么时间，以什么样的方式，对什么代码，和什么人，做事。

因此，怎么做，在第三节Code Review 实践过程来说，这里对其他要素，进行了抽象总结为2个方面：review对象，review时机。

2.1 Code Review 时机

先来看，我们的研发流程：

一般的需求研发流程中，开发主要主导方案评审和技术开发两个阶段。所以，我们的Code Review 肯定是应该在测试阶段之前完成的。一般要求代码在测试阶段后，最多只会有些小的bugfix，如果存在较大改动，理论上是因为被QA同学打回重新自测；但是，对于Code Review而言，可能会存在大量代码调整，为了保证代码在测试之后保持稳定性，需要在提测之前完成代码review和修改。

明确了Code Review 在提测之前完成，那么整个开发过程中，什么节点发起 Code Review呢？

一个需求，可以分为大需求和小需求。大需求可能涉及大量代码调整和开发，一次CR时间太长，效果会达不到预期，因此，可以控制一次CR的代码量，按照功能拆分多个CR，毕竟我们的开发也是逐个功能来开发的。自然而然，按照开发排期节奏，一个完整功能开发完成后，推PR/MR/CR平台，进行代码review。如下：  

额外建议：一次MR，建议完成一个单一的功能，并且这个功能有完整详细的背景和目标，这样，审查代码的同学，可以很高效的完成代码review。并且，开发同学，理论上对于这块代码的修改和调整，也不会影响到其他功能的继续开发。

2.2 Code Review 对象

review对象主要包括：review的代码对象，和review的参与对象：包括发起者，和审查者。

2.2.1 Code Review 代码对象

什么代码需要进行review，其实挺难的。最简单的方式，是所有代码都进行Code Review，但是显然这样太耗时间，投入产出比太低。因此，就需要根据具体团队的情况来具体抉择了。一般而言，对于以下属性的代码需要进行review：

• 新人写的代码。很多时候，新人的代码风格和现有团队会不一样，并且对当前业务代码熟知程度一般，因此，这部分代码需要进行review。
• 核心功能的代码。核心链路核心功能的代码，一般会长期维护，如果代码写的不符合约定的规范，后期其他同学维护成本就会增加；此外，虽然核心功能测试配套设施比较多，但是如果通过Code Review 可以发现问题，那么规避测试阶段未发现带上生产环境；最后，核心代码的修改，通过review，可以让相关同学周知。
• 非业务owner/co-owner的代码。一般一个系统或某个功能模块，有对应负责的owner，当非业务owner去开发修改功能代码时，可能会改动一些重要的类或方法，没办法进行评估，毕竟，众所周知，一个业务系统代码里面，总是会存在一个匪夷所思的业务需求判断分支。

除此之外，在有足够时间或者团队对于Code Review足够认可的情况下，以下场景下的代码也可以考虑进行review：

• 核心组件或工具类。团队内部一般会抽象一些工具类或组件，这些工具未来都会在团队内部推广使用，因此，建议进行代码review。其一，是通过代码review发现一个开发和自测阶段未发现的问题点；其二，是一个很好的技术推广和技术交流的机会，一个组件或工具的开发和抽象，一般都是具有技术挑战的，对于团队内技术学习氛围是个很好的机会；其三，同时对组件和工具使用场景和方式进行介绍，方便后期使用过程中避免踩坑。
• 精心的设计和实现代码。有些功能实现的时候，做了非常好的抽象和模式使用，期待和大家一起交流。这对于一些实现复杂或者抽象程度高的模块，是个非常好的机会去学习，尤其是对新人是个非常难获取的示范。
• 期待更好实现建议的功能模块。对于有些同学，虽然代码写完了，但是觉得应该有更好的实现，或者有些代码实现可能有并发、多线程安全、分布式问题时，建议发起Code Review。

2.2.2 Code Review 参与对象

如果参与Code Review的都是新人，那么基本上而言，这种review效果并不是很好，除非团队全是新人——这种情况下，一定要先讨论&产出团队内部的代码规范。

对于发起Code Review的同学，即，以上代码对象的开发者。参与评审的人员：

• 该业务模块的owner/co-owner。必须参加评审，对业务和代码最了解的人，必须了解任何人对该模块的改动，毕竟后期的维护和问题处理，都是业务模块的owner来负责。
• 新同学的导师/师兄/有伴。一般而言，新入职的同学，不管是应届生还是社招新人，都有会一个mentor负责协助和指导，那么新同学进行Code Review的时候，mentor就肯定需要参加给与意见和建议。并且，如果三个月内的功能开发出现线上问题，一般是mentor来担责的。
• 团队技术资深同学。互联网公司，会有各种并发和分布式场景问题，这些问题在一般的单测和测试环境都可能发现不出来，那么很多时候，技术资深的同学，可以利用多年的经验和技术实力，来发现隐藏的bug，协助开发同学在上线前解决。

此外，针对优秀的代码 发起的review，可以拉更多的同学进行review，提升发起者的技术影响力，同时，也能帮助团队其他小伙伴成长。

三、Code Review 实践过程

对齐了Code Review价值和目标，清楚了需要参与Code Review对象和时机后，接下来就是如何落地。

在决定执行Code Review的时候，项目需求排期，需要将这部分时间加进去。尤其是一个复杂的稍大型项目，可能因为CR问题较多，花费大量时间去修改，最终影响项目进度。同时，很多时候，由于项目进度的压力，导致CR的非阻塞性问题，一些都没有fixed，就直接提测上线。
此外，从上节介绍的时机来看，在拆解和排期需求的时候，如果项目大，需要考虑如何最优的方式安排功能开发节奏和Code Review最匹配。

整体流程如下：

先分三步骤来看：CR前准备，CR评审和fix，CR后结尾。

3.1 Code Review 发起准备

在发起Code Review之前，需要做一些准备工作，来提高Code Review效率。

1. 技术方案评审。对于发起CR评审的改动，对应的技术方案是应该事先评审，或者技术方案文档，应该提供出来。当前，很多时候，我们发现，参与评审的同学，对于设计方案的不清楚，导致对代码进行Code Review的时候，无法很好的理解代码思路，最终对review的效率和效果都是大打折扣。
2. 单元测试等手段，保证功能自测通过。一个基础功能都有问题的代码，是不应该发起Code Review。CR 不是让大家给你找功能bug的，而是发现不影响功能的隐藏bug。当然，如果测试用例严谨，可能一些隐藏bug也不应该出现，只不过当前我们对测试边界用例完整度要求不高而已。
3. 静态扫描工具，例如SonarQube平台。静态扫描工具，可以发现一些明显的代码问题，比如资源没有关闭，代码缺少一些元素，空指针判断等IDE无法直接提示的，不影响编译的问题。
4. 代码检查插件工具，例如Alibaba Java Coding插件。可以对java语言中，一些代码开发规范和容易出问题的编码姿势，给出提醒。例如，线程池需要自己原始构造，等。
5. 代码注释。提交CR之前，最好检查下，核心流程或者复杂流程中的代码注释是否完备。变量和方法的取名要求，有时候并不合理，每个人有自己的风格和词库。因此，注释是让所有人明白含义的最好方式。

准备好以上工作之后，如果需要发起会议review，则在会议代码讲解之前，将需求和技术方案再简单过一下，大家保持信息一致。如果是gitlab review，则将设计文档和需求文档，贴在review 描述中。

ok，接下来，可以在MR平台，或者PR平台发起 Code Review请求，邀请相关同学对你的代码进行review。

3.2 Code Review 评审阶段

这个阶段是最核心的部分，前置的准备操作，是为了这个阶段最高效。

首先，我们期望，审查者可以确保随着时间的推移，CR的质量不会使代码库的整体健康状况下降。也就是说，审查者，不能因为说之前代码已经这么随意了，对新代码的CR就要求低了。众所周知，代码库健康状况会随着时间的而下降，这是个定律，但是如果不能如此要求CR，代码质量管理迟早会失败。

其次，需要明确，代码评审，我们并不要求完美的代码。参考前面一条，如果本次的CR不会导致代码整体质量下降，则可以同意本次MR。不过，这显然是建立在代码没有bug，只是实现姿势不是最优的情况下。

基于以上两点，来看我们如何操作Code Review 。

3.2.1 分支管理

目前，我们的项目研发分支使用，简图如下：

但是这种分支，对于一个需求多次MR/CR操作，并不是很友好，基于目前的分支习惯，我们可以拆出一个项目分支，整体如下：

首先，我们从master 拉分支作为开发分支，开发分支完成一个独立功能点，可以提交PR/CR/MR之后，从开发分支A1上拉一个项目分支A，我们针对项目分支A进行Code Review操作。开发分支A1可以继续开发其他的功能，项目分支A完成CR和代码fixed之后，将修复完的代码，同步到开发分支A1。依次类推，最终确认开发完和CR完成之后，我们已项目分支A提测。

因此，开发分支A1主要为了同步开发使用，毕竟就目前而言，Code Review一般不会随时进行，但是这段间隔时间，项目功能需要持续开发。

3.2.2 Code Review流程

首先，我们建议代码审查者，可以先阅读G家的规范：代码审查者指南；
代码开发者可以阅读：代码开发者指南。

Code Review 主要包括三个流程：

3.2.2.1 Code Review 发起操作

Code Review主要两种形式：会议review 和 gitlab review 两种，我们建议绝大部分情况下使用gitlab review形式。如果发起者发现gitlab review的反馈中，存在很多重复的问题，并且这些主要是对设计和实现理解的问题时，可以做一个总结，然后拉个会议review一起讨论下。当然，最终发起还是需要在gitlab上面有个MR/CR的记录。

在发起CR/MR的时候，需要将需求文档，设计文档，以及改动文档放在提交申请的 描述里面；如果改动较小，可以将本地改动点文字形式描述下。

依赖commit log来了解改动是一个比较推荐的方式，但是就目前开发习惯而言，这种要求可能比较难以落地。所以，降低门槛，将改动点已文字形式放在申请描述中。

3.2.2.2 Code Review 审查操作

Code Review申请发起后，代码评审同学就可以到gitlab或者其他CR工具上进行代码review。对于审查同学的要求，查看代码结构命名是否规范，代码逻辑是否有隐藏bug，代码性能是否存在问题，代码可扩展可复用性是否过低，等等。每个人的标准和经验都可能不一样，因此给出的review评论也会存在差别，但是有一些建议如下：

• 设计和实现一致性。提交review的时候，会有相关的需求设计和技术设计，包括改动点。从需求上可以评估到本次的改动和需要提供的功能，如果本次改动超过了这部分需求，则需要开发者给出原因，评估是否实现上存在问题。技术方案开发前已经评审，代码评审的时候需要看方案和本次改动点是否一致，如果存在冲突，需要评估是否存在问题。
• 代码逻辑复杂性。我们认为越简单的代码，越容易测试，后期维护也越简单。因此，如果存在复杂的逻辑或者代码块，需要评估是否有简洁的实现，是否可以拆分+组合模式来实现，或者评估下是否存在过度实现。代码复杂问题还是非常普遍的，因此，代码评审的时候，这块是重点，也是考验评审人员经验的地方。
• 注释和文档的关注度。核心节点需要有注释，一些特殊的业务逻辑，也需要注释，复杂的策略算法等部分，可以链接对应文档。代码注释，很多时候，可以帮助理解业务，如果代码评审着不能轻易看懂这块逻辑，要么写的过于复杂，要么需要开发者提供必要的注释支持。

当然， 关于代码正确性相关的审查，是最基础的诉求。

此外，我们对于审查review的评论，也有些建议：

• 保持友善。
• 对于代码问题，给与解释和建议。
• 给与适当指导。有的时候，有些优化方案可能涉及设计模式，抽象复用等比较难以想到，这个时候，需要给以适当的指导，可以是示例代码或者面聊等。

3.2.2.3 开发者fix操作。

Code Review评审的产出物，就是各种代码修复建议，开发者需要针对这些问题进行修复和优化。对于有疑问的地方，可以进一步讨论。

开发者在修复完问题之后，继续提交，并且在commit log里面，给出修复的问题是啥，怎么解决的，思路是什么，等等。

所有的问题修复，都在项目分支，当问题被修复完成之后，代码评审者，需要针对修改的地方，进行再一次的review，确保修复都是符合预期的。从而形成一个有效的闭环。

3.3 Code Review 结束工作

发起的一次Code Review，对所有review问题完成修复之后，CR就结束了。

最终所有代码都应该在 项目分支，把项目分支作为提测分支，发起提测，后期的测试阶段bug修复，就直接在项目分支进行fix。

在测试完成之后，将代码合并到master分支上，直接gitlab上将MR通过即可。

四、Code Review 工具指南

4.1 Code Review 静态扫描

在发起Code Review之前，需要对代码进行静态扫描，一般我们推荐使用sonar工具，进行静态扫描，扫码完成之后，就可以在sonar页面上看到一些代码健康度，以及相关代码建议了，原则上，需要把所有建议都修复掉。例如：

4.2 Code Review 插件

首先，在gitlab 上构建一个Merge Request，如下图：

然后，我们可以在gitlab 或者使用 idea 插件Merge Request Integration CE 来完成代码审查工作。

推荐使用Integration CE 插件，用起来比较方便。下面给个插件使用说明：

安装插件完成之后，先配置server地址和令牌

然后，在idea下方，打开看到自己的code review列表：

然后，我们就可以开始通过idea 客户端进行code review了

五、总结和讨论

每个团队都有自己的Code Review规则和流程，本文参考借鉴了公司内外的一些cr经验分享，结合团队内部的目标和问题，梳理了一份落地的方案。

需要注意的是，每个团队的情况都存在不同，业务属性、发展阶段、当前目标、内部成员素质，技术储备等等，都存在千差万别；并且不同需求紧急程度、系统定位、代码结构、原始代码库健康度，也都存在差别。
因此，在执行过程中，会根据不同场景进行调整和伸缩，发挥好Code Review最大的价值，同时，也着眼于当前核心目标，助力业务快速迭代上线。