如何在团队中做好 Code Review

标签: dev | 发表时间:2019-12-29 00:00 | 作者:
出处:http://itindex.net/relian

一、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是可以强制Code Review的,支持Develop、Reviewer、Approver三种角色支持对每个Project配置不同的Code Review的人员以及权限。

如果要根据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的Code Review教程:https://ken.io/note/gitlab-code-review-tutorial

2、制定开发规范

没有规则,就没有执行。规则中首当其冲的就是开发规范。规范中建议包含:

  • 工程规范(工程结构,分层方式及命名等等)

  • 命名规范(接口、类、方法名、变量名等)

  • 代码格式(括号、空格、换行、缩进等)

  • 注释规范(规定必要的注释)

  • 日志规范(合理的记录必要的日志)

  • 各种推荐与不推荐的代码示例

如果团队人数较少,项目的工程复杂度较低,可以自行制定规范。毕竟适合团队的就是最好的。如果团队有一定规模,且还会不断扩张,还是建议根据大厂的规范进行制定,或者是直接采用。

Java开发手册:https://github.com/alibaba/p3c
Google代码风格指南:https://zh-google-styleguide.readthedocs.io

(涵盖:C++、Python等)

3、制定流程规范

  • 确定Code Review实施环节

Code Review建议是放在代码提交测试前,也就是开发人员完成代码开发及自测后将代码提交到测试分支时进行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的执行

  • 定期分享

我们是期望Code Review可以让工程师之间互相学习的,那么对于一次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的耗时是团队内最高,有可能是有某次代码是周五晚上提交的Code Reviw,这单次Code Review的耗时就会超过48小时。也有可能是对应的Reviewer是团队新人,要通过相关业务项目了解对应Project的承担的职责及代码,这是个学习的过程,自然耗时加长。

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

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

三、保证Code Review质量的关键

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

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

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

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

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

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

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

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

四、备注

附录

  • Java开发手册:https://github.com/alibaba/p3c

  • 基于GitLab的Code Review教程:https://ken.io/note/gitlab-code-review-tutorial

  • Google代码风格指南:https://zh-google-styleguide.readthedocs.io

  • Jenkins+Sonar执行代码扫描:https://ken.io/note/jenkins-maven-java-sonar-integration




题图作者:athree23

授权协议:CC0协议


相关 [团队 code review] 推荐:

如何在团队中做好 Code Review

- - IT瘾-dev
一、Code Review的好处. 想要做好Code Review,必须让参与的工程师充分认识到Code Review的好处. 无论是高手云集的架构师团队,还是以CURD为主的业务开发团队,大家的技术能力、经验都是有差异的. 通过Code Review,对于同样的功能实现,有经验的工程师可以给经验尚浅的工程师提供合理的优化建议.

聊聊Code Review

- - 梦想风暴
hopesfish评论《 那一点的调用》时,问了一个关于Code Review的问题:. 想请教一下,TW的筒子是如何做code reivew或者鼓励客户做code review的. 我在翻阅博主的帖子的时候,似乎对这块没有特别强调,而是更多偏重于TDD,我觉得TDD的问题是一碰到没有责任心的程序猿,就很容易流于形式了.

Java Code Review清单

- - ImportNew
使用可以表达实际意图(Intention-Revealing)的名称. DRY(Don’t Repeat Yourself)原则,(拒绝重复). 用代码来解释自己的做法(译者注:即代码注释). *参考自: http://techbus.safaribooksonline.com/book/software-engineering-and-development/agile-development/9780136083238.

我的code review规则

- vento - 我的宝贝孙秀楠 ﹣C++, Lua, 大连,程序员
1) 是否有语法错误,编译错误,编译警告. 做法:下载最新代码,将编译警告级别提升到最高,检查output信息. 2)是否符合需求,完成requirement文档要求的内容,不能多,也不能少. 注意:即使发现有问题代码,如果与需求关联不大,不要涉及. 应该让每次enhancement和bug fix最简洁,牵涉范围最小,影响到组件最少.

Code Review那些事儿

- - 非技术 - ITeye博客
       曾经有一段 垃圾代码放在我的面前,我没有拒绝,等我真正开始接手的时候我才后悔莫及,程序员最痛苦的事莫过于此. ---------改编于周星星的经典台词.       虽然有点夸张,但编码界确实大大存在这种情况,每当接手别人的代码,都有一种想重新写一遍的感觉,等到别人再来接手你的代码时,同样的感觉.

代码审查(Code Review)清单

- - 博客 - 伯乐在线
代码审查可以帮助提高代码质量,避免由于代码习惯而造成的 bug. 下面列出的这些要点因该可以作为大部分代码审查的指导,如果是 Java 应用的话,这些建议应该被视作最佳实践. Javadoc 应该在每一个类和方法中添加. 如果是修复某个 bug,应该添加 bug ID. 走捷径的方法或者复杂的逻辑要有解释.

从Code Review 谈如何做技术

- - 酷 壳 - CoolShell.cn
(这篇文章缘由我的微博,我想多说一些,有些杂乱,想到哪写到哪). 这两天,在微博上表达了一下Code Review的重要性. 因为翻看了阿里内部的Review Board上的记录,从上面发现Code Review做得好的是一些比较偏技术的团队,而偏业务的技术团队基本上没有看到Code Review的记录.

我们该如何做好Code Review?

- - SegmentFault 最新的文章
我们该如何做好Code Review?. 午后的阳光,静静地照在你的脸上. 这时候配上一杯82年的java,脑子一片灵光闪过,呃......上午刚写完需求,下午好像没什么事了,不如看看自己写的代码. 至此,一场Code Review也就拉开序幕了. Code Review,即代码审查,是指对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术.

测试技术中CODE REVIEW的重要性

- - CSDN博客推荐文章
        [近期关注App自动化测试,欢迎交流,本博客文章版权归作者所有,转载请联系]     .         最近有网上的朋友向我咨询作为测试员是否应该跳槽,   首先我觉得应该向大家介绍一下什么是测试工程师,  什么是测试员,   在国内的一些中型企业并没有特别的指明.   这里测试工程师主要指测试开发工程师, 主要包括两类,  其一是测试软件开发的工程师,  其二是自动化测试脚本开发和维护的工程师,   而测试员主要指单纯编写/管理测试用例,  或是手工测试人员, 一些国内的大中型网络视频公司仍然在用纯手工测试,我感觉到很汗颜.