代码改变世界

如何在团队中做好Code Review

2019-12-23 10:40  KenTalk  阅读(5412)  评论(22)  编辑  收藏  举报

一、Code Review的好处

想要做好Code Review，必须让参与的工程师充分认识到Code Review的好处

1、互相学习，彼此成就

无论是高手云集的架构师团队，还是以CURD为主的业务开发团队，大家的技术能力、经验都是有差异的。

通过Code Review，对于同样的功能实现，有经验的工程师可以给经验尚浅的工程师提供合理的优化建议。经验尚浅的工程师可以通过阅读优质代码，快速学习相关技术运用的最佳实践。如果大家技术实力相当，可能就是互相刷新思想了。

你有一个苹果，我有一个苹果，彼此交换一下，我们仍然是各有一个苹果；但你有一种思想，我有一种思想，彼此交换，我们就都有了两种思想，甚至更多。

2、知识共享，自动互备

在大部分团队，尤其是采用服务化架构以及微服务架构的团队，通常都是1个开发人员负责多个服务/项目（Project），如果没有Code Review，那么项目中所涉及的架构知识，或者业务知识，就只存在于项目执行过程中产出的架构文档，以及核心流程、功能的说明文档了。

文档可以帮助其他工程师了解服务/项目的情况，但通常其他工程师不会主动去阅读这些文档，等到真的要维护别的工程师写的代码，文档的完整性往往没有最初的效果好了，文档跟代码实现的匹配度也会下降。

Code Review的过程，就是根据提交者的描述阅读代码的逻辑，看代码实现是否跟描述一致。在这个时候，Reviewer就必须阅读文档，知识的传播性就更好，也基本上不会出现只有1个人了解某个项目的情况了。

3、统一风格，提升质量

如果要给代码质量分一下等级的话，那应该是：

可以编译通过->可以正常运行->可以测试通过->容易阅读->容易维护。那么，通过Code Review的代码最起码可以达到易阅读这个级别。

要做到易阅读，可不是说只要有Code Review这个环节就可以了，还要有相关的规范，让大家按照同样的工程风格、编码风格去构建项目和编写代码。统一风格一方面是让大家无论是维护项目还是阅读代码，不用互相适应各自的编码习惯，另外也是给Reviewer一个Code Review的基本依据。

发现Bug不是Code Review的必需品，而是附属品。至于那些低级的问题/bug交给代码扫描工具就可以了，这不是Code Review的职责。

二、推动Code Review落地执行

1、选定工具

可以用来做Code Review的工具很多，这里主要介绍相对主流的Gerrit、GitLab

  • Gerrit

Gerrit是Google开源的代码审查工具，Gerrit也是一个基于Git构建的版本管理工具，Gerrit支持将其他Git仓库的代码跟Gerrit自己的仓库做同步。所有的代码审查的操作以及权限控制都是在Gerrit自己的仓库上进行的。

Gerrit是面向代码审查来构建的，所以在代码审查的权限控制，以及功能上都是非常完善的。

Gerrit是可以强制CodeReview的，支持Develop、Reviewer、Approver三种角色支持对每个Project配置不同的CodeReview的人员以及权限。

如果要根据Gerrit的数据做一些统计报表，就直接访问Gerrit的数据库，如果功能上不满足要求，反正是开源的，有Java研发团队就可以自己定制

总之，Gerrit的Code Review功能是非常完善的，缺点可能就是UI、交互太老了以及平台的管理功能较弱。

  • GitLab家族

GitLab是基于Git构建的源代码管理系统，基于GitLab构建的 GitLab.com 是仅次于 GitHub.com 的在线源代码管理平台。

GitLab分GitLab CE（社区版）和 GitLab EE（企业版）两个版本，开源的社区版功能相对会弱一点，但是免费使用，可以自由部署、定制、维护。企业版功能强大，但是需要收费的。

GitLab可以通过MergeRequest来Review代码，也可以做到强制CodeReview，社区版支持Develop、Reviewer两种角色，企业版支持Develop、Reviewer、Approver三种角色，可以给给项目/组分配不同的角色(Master、Developer)来控制Merge代码的权限。

如果需要根据GitLab的数据做一些统计报表，GitLab提供了非常友好的restful API，如果要定制化，建议是通过API来做定制化的工具，不受编程语言限制。

GitLab的Code Review的功能没有Gerrit功能完善，但是GitLab附带的文档功能、以及GitLab完善的管理后台都要比Gerrit更好，如果要做CI/CD，GitLab的社区版几乎是最佳选择

  • Gerrit VS GitLab 综合对比

工具

权限 控制

UI 交互

源代码 管理

可维护

数据 统计

工具 配套

Gerrit

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐

⭐ ⭐ ⭐

⭐ ⭐

⭐ ⭐ ⭐ ⭐

⭐ ⭐

GitLab社区版

⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

GitLab企业版

⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐ ⭐

⭐ ⭐ ⭐ ⭐

Gerrit强项只有Code Review的控制，GitLab的功能更全面，但GitLab的企业版是收费的。所以，综合来说，我更推荐GitLab社区版

基于GitLab的CodeReview教程：https://ken.io/note/gitlab-code-review-tutorial

2、制定开发规范

没有规则，就没有执行。规则中首当其冲的就是开发规范。

规范中建议包含：

  • 工程规范（工程结构，分层方式及命名等等）
  • 命名规范（接口、类、方法名、变量名等）
  • 代码格式（括号、空格、换行、缩进等）
  • 注释规范（规定必要的注释）
  • 日志规范（合理的记录必要的日志）
  • 各种推荐与不推荐的代码示例

如果团队人数较少，项目的工程复杂度较低，可以自行制定规范。毕竟适合团队的就是最好的。

如果团队有一定规模，且还会不断扩张，还是建议根据大厂的规范进行制定，或者是直接采用。

Java开发手册：https://github.com/alibaba/p3cundefinedGoogle代码风格指南：https://zh-google-styleguide.readthedocs.io （涵盖：C++、Python等）

3、制定流程规范

  • 确定Code Review实施环节
image

CodeReview建议是放在代码提交测试前，也就是开发人员完成代码开发及自测后将代码提交到测试分支时进行Code Review。毕竟，如果测试通过后再进行Code Review，如果需要代码变更，势必会增加测试的工作量，甚至影响项目进度。亦或是顶着项目上线的压力，干脆“以后再说”了

以通用的Git Workflow来说，那就是把Code Review放在Feature分支合并到Develop分支时了。

  • 制定角色行为规范

角色

规则

Developer

1、一次提交的功能必须是完整的 2、默认细粒度提交（以独立的方法/功能/模块为单位）。如需粗粒度提交，需提前跟Reviewer沟通确认 3、Commit Message中要清晰描述变更的主题 必要时，可以以链接或者文件的形式附上需求文档/设计文档

Reviewer

1、不允许自我Review并Merge代码 2、Review不通过打回前需跟Developer说明原因并达成一致 3、Review不通过需明确填写打回的原因 4、单次Review时长需控制在2分钟~2小时内完成（特殊情况请说明原因）

Approver

1、审批不通过需注明原因 2、审批时长需要控制在1小时以内 3、对于放行的非质量问题，需持续跟进

这样规范，主要是为了：

  1. 控制提交Code Review的代码的粒度
  2. 控制单次Code Review的时间
  3. 提升Commit/MergeRequest描述的质量，减少沟通成本

这样，我们就可以通过细粒度高频次的方式尽可能利用工程师碎片化的时间进行Code Review，一定程度上保证Code Review的效率。

毕竟，粗粒度甚至是集中式的Code Review，时间上难以把控。发现了问题的时候，修复的成本也往往更高。

3、分享与统计

有了工具、开发规范、流程规范，就可以指引参与的工程师参与Code Review，那么我们也要对Code Review的过程以及结果进行检验，毕竟不进行检查/验收的规则，是无法达到预期效果的。

Code Review毕竟不是数学题，我们无法通过简单的计算去验证。所以我们要通过侧面验证，来帮助Code Review的执行

  • 定期分享

我们是期望CodeReview可以让工程师之间互相学习的，那么对于一次Code Review通常只有参与的2-3个工程师有互相学习的机会，那么在这个过程中学到的知识，定期的分享出来，既可以加强知识的流动，又可以检查大家究竟有没有在Code Review过程中学习到知识，或者有没有认真的进行Code Review

至于分享的内容，可以是开发规范中的范例代码，也可以是规范中的正例代码，也可以是针对某个功能实现的最佳算法/最佳实践，也可以是Code Review过程中的争议代码，也可以是自己踩过的坑。

总之，Code Review之后的代码分享，不但可以加强知识的流动，还可以检验Code Review的效果。

  • 数据统计

为了在一定程度上保证Code Review的效率，我们在规范里是要求参与的工程师：

  1. Developer控制提交Code Review的粒度，或者控制每个Commit的粒度
  2. Developer要准确清晰的描述所提交的代码
  3. Reviewer&Approver要在规定时间内完成Code Review

这些情况纯粹靠人工是无法检验的，还是需要有一定的数据统计。

如果用Gerrit，可以查询Gerrit的数据库，里面会有Code Review的信息，

如果用GitLab，可以通过WebHook或者restful API获取Code Review信息

我们可以做成报表，来展示Code Review的情况：

  1. 每人每周Code Review所消耗的时间
  2. 每人每周被Code Review所消耗的平均时间
  3. 超过规定时间的Code Review情况
  4. 代码提交描述字数过少的情况
  5. 等等（根据自己的需要来）

以上情况只是Code Review的侧面反馈，用来帮我们发现Code Review执行过程中可能出现的问题。不过，出现问题并不意味着Code Review的质量/效率一定受到了影响。

比如，工程师A被Code Review的耗时是团队内最高，有可能是有某次代码是周五晚上提交的CodeReviw，这单次CodeReview的耗时就会超过48小时。也有可能是对应的Reviewer是团队新人，要通过相关业务项目了解对应Project的承担的职责及代码，这是个学习的过程，自然耗时加长。

又比如工程师B提交的代码描述文字过少，可能就是中间件团队对某些基础组件进行升级，或者安全团队要求升级某个依赖的开源组件，以修复某个安全漏洞。

但是通过这种的数据，可以让Code Review的情况直观的展示出来。来发现大家执行过程中需要优化的事项， 不断帮助大家完善规则，做好执行。

三、保证Code Review质量的关键

1、工程师对研发规范的认真学习

无论Code Review的工具以及流程是怎么样的，都少不了开发规范作为支撑，毕竟我们期望Code Review达到的效果之一就是，团队中的工程师可以写出像规范中描述那样的高质量代码。

工程师对研发规范的掌握程度，决定了自己编码代码的质量，也决定了自己Review通过的代码的质量。所以，无论如何，加强对研发规范的学习和理解，都是保证Code Review质量的重中之重

2、资深工程师的认真对待

Code Review目的是帮助工程师交流和学习进步的。无论是技术能力还是编码习惯，亦或是业务知识。无论规则怎么制定，终究还是需要参与的工程师来执行，如果大家互相睁一只眼闭一只眼，互相降低要求，那么执行的效果一定会打折扣。

虽说三人行必有我师，但收益最大的一定是经验(技能/业务知识)尚浅的工程师，收益最低的一定是团队中最资深的工程师。而恰恰经验尚浅的工程师的收益大部分都要来自资深工程师的付出。

所以，一定要跟资深工程师最好沟通，让他们严格要求，不能对经验尚浅的工程师放水，以帮助他们提升编码能力以及业务知识。

这也可以减少甚至避免他们为经验尚浅的工程师的代码“善后”。

四、备注

附录

本文首发于我的个人博客：https://ken.io/note/how-to-do-code-review-in-a-team