你关切的Code Review三大问题，我以业务实践作答

近日，在极狐 TechTalk 直播上，极狐(GitLab) 高级解决方案架构师杨周分享了高效 Code Review 秘籍以及 Code Review 在实际业务场景中的最佳实践，并手把手带大家基于极狐GitLab 进行了 Code Review。

 

以下内容整理自本次分享，干货爆满，分为上下两期：

 

上期回顾👉代码质量问题、分支创建与评审、Git 规范、安全漏洞；

这期继续分析👉代码规范、Code Review 实践中大家最关注的几个问题。

代码规范

常见的代码规范问题有哪些？

不同公司有不同代码规范

很多公司制定了自己的代码规范，甚至是打印出来贴在墙上。Code Review 规范到底要不要去人工干预？

首先，我们看一下这段 Java 代码，这里有 3 个问题，Java 书写规范一般要求：

• 类名和括号应该在同一行；
• 函数名应该用驼峰；
• 缩进应该用空格，而不是 tab。
   

不同语言的规范各不相同

我们再来看一个 Go 的代码。很多同学可能要抢答了，一眼看到这里的数据不对，底下俩空格上面是 tab？非常遗憾，这其实是对的。

大部分语言都是空格缩进，但 Go 官方要求用 tab……go fmt hello.go 会强制转换。这也是非常新颖的做法，就是语言官方自带规范，所以大家不要带着一个语言的习惯去评审另一个语言。

魔法数字难以维护

最后来看这个 PHP 代码有什么问题。这里有一个大括号换行问题，下面又有一个换行。

其实这些都是对的。PHP 很流行的规范是 PSR-2，它定义的是类后面大括号必须换行，和 Java 恰恰相反，但 if else 不换行。

大家要注意一下，不要搞得精神分裂了。

所以，这段代码超级换行没有任何问题，唯一的问题是魔法数字。if else 使用了 1 和 2 难以理解，这里应该用常量来表示。还有它的 if else 嵌套的太深了，复杂度太高，难以理解，难以维护。

代码规范有很多种问题，比如每个人的书写风格和习惯不同，人工去检查缩进换行浪费大量时间，更别说老项目，有很多的问题导致代码规范无法落地，这些都是业界常见的问题。

极狐GitLab 是怎么解决这些问题的？

首先，我们推荐大家采用业界知名的代码规范，而不要轻易自己造规范。因为除了写公司的代码之外，我们可能还会参加很多开源项目。开源项目一般都采用业界知名的规范，如果你自己造了一个跟大家都不一样的规范，做别的项目会比较麻烦。

上图是几家知名规范，比如 Java 有 Google 规范、PMD 规范，Go 语言有官方规范，PHP 有 PSR 规范。

这些规范主要分成两大类：风格规范和复杂度规范。

• 风格规范规定缩进、换行、空格这些规范；
• PMD 限制的是复杂度，复杂度不能大于10，这是默认值，如果团队目前复杂度比较高，可以自己去修改。另外，行数不能超过 60 行，否则太复杂了；类不能超过 1000 行，否则违反了面向对象的单一职责。

一般推荐大家先落地风格规范，等所有成员都上手了，再落地复杂度规范，再逐渐收紧，比如先调成 15 复杂度，逐渐收到 10 以下，甚至收到 5 以下。

业界知名代码规范都有一个特点，全部是有配套的命令行工具，而不是一个文档。有的大厂有自己的规范，发出 PDF 给大家看，这样是不合适的，而应该是一个配置文件加命令行工具，能让大家一键执行，才是一个合格的规范。比如 Java Checkstyle、 Code Sniffer、前端 eslint，连 Markdown 文档也是有规范的，比如阮一峰规范，用 Lint-md 来扫描。

怎么在团队里落地规范？

我们以 Java 为例来看一下，比如 check style，引入 Maven 插件，从 check style 官方开源项目里下载 xmail 放到指定目录里，执行 ./mvnw checkstyle:check，就会扫描出来有几个问题，在哪一行。

如果检查失败了，会有一个非 0 的退出码，对 Linux 比较熟的同学应该知道，退出码是非 0，意味着这是一个错误，我们就可以捕获它。

如果你不是用 Maven 构建，而是用更先进的 Gradle 构建，也类似引入 check style 插件，下载规范文件。

很多开发者会质疑：IDE 自带了代码规范，为什么还要下载代码规范放在 Git 里，还要配置命令行工具？

因为团队人多了之后，很难统一开发工具，有的人觉得 JetBrains IDEA 一年 1000+ 比较贵，用免费的 VSCode，结果各种编辑器默认的规范不同。而 Git 里的规范可以确保大家都可以指定同一个规范。

命令行工具也有大用处。

刚才说到有一个检查规范的命令，退出码非 0 的话，可以把它放在持续集成里面自动跑起来，这样就可以实现自动化的检查代码规范了。这也是持续集成的定义——通过在代码持续合并的过程中做一些自动化的工作，来实现质量内建。

下图是极狐GitLab 的持续集成配置，非常简洁，就这几行代码就可以跑起来了。

首先我声明了一个Java 17 的镜像，很多同学可能在用 OpenJDK， 一定要注意 OpenJDK 安全漏洞非常多，已经被 Java 官方废弃了，现在大家比较常用的是 Eclipse 镜像，这里就不展开说了。

接下来，执行刚才的 checkstyle 命令，然后在极狐GitLab 设置里勾选流水线必须成功。

因为有退出码，假如有任何不规范，就会导致整个流水线失败并终止，这样操作可以确保项目非常干净。如果流水线失败了，有一个叉号，自然就不能合并。

但我们并不清楚，流水线为什么失败了，是编译失败了，还是代码规范哪一行有问题？针对这个问题，极狐GitLab 提供了一个更好的方式。

Java 代码规范和极狐GitLab MR 打通

同样以 Java 规范为例，我们先引入 checkstyle 插件，下载代码规范，需要声明检查规范允许失败，失败后要把报告采集上来，并生成一个代码规范的报告，但它不是极狐GitLab 的格式，还需要通过 Maven  validate 对格式进行转换。

采集上来之后，在合并请求里就能看到多出来一个叫做“代码质量模块”的区域，这样开发同学一提交代码就能看到代码有规范问题，就可以自己修复。

上图可以看到历史遗留问题有几千个，大家暂停手头工作一起来修也不现实，理想的方式是做成增量代码规范，极狐GitLab 已经实现了增量代码规范报告。

极狐GitLab 增量代码质量报告

当我们第一次配置好规范后，在主分支或者 develop 公共分支上跑一次，产生了 8000 个问题，存起来，下次其他开发人员拉取临时分支的时候，又进行了一次全量扫描，可能也有 8000个问题，极狐GitLab 会对这两次问题自动进行 diff，从而获得一个增量代码质量报告，这个配置方式是最简单、最友好的。

增量报告的效果非常好，可以看出只有新写的代码带来了 12 个新问题，而几千个老问题不会再显示，当你改到某一行老代码，顺手修掉老问题，这里才会显示。这个开源工具能转换多种扫描报告，比如 PMD、Checkstyle。

总结一下，Code Review 时，完全不用操心代码规范和复杂度。极狐GitLab CI 结合业界热心开发者做的格式转换工具，就可以非常丝滑的打通，全自动的拦截代码规范。

Code Review 是否影响老代码？

随着项目越来越大，没人敢改老代码，有的人就把函数复制一份。这样虽然没有影响老代码，但是影响整个项目的可维护性，这是很多老项目必然面临的一个无解的问题。

另外，回归测试也越来越久，假如要改一个紧急的 bug，想上线也要回归，这么大的项目回归测试需要很久。那多花钱招一些测试人员行不行？

实际上是解决不了问题的，业界已经踩过这个坑了。因为 bug 多的根源是开发人员写的代码质量不高，而不是测试人员的问题。只有在开发阶段就确保代码质量，才能更好的规避这些问题，也就是进行自测。

很多开发同学都会用 Postman、Curl 或在浏览器里去测试，这样行不通的。因为你第一次写的时候，有耐心测了 4 次，比如左边代码有 4 种情况，测了 4 次，但第二次改的时候，可能就不想测这么多次了，或者别人改的时候也不会帮你测所有情况。于是，低成本、低质量必然的结局就产生了，这在国内非常普遍。

但如果严格要求任何人改一个字母、一个空格都要去测试所有情况，成本将高到难以接受。上图里的 4 种情况算少的，往上组合可能是 4×4×4 指数级的数量。

作为一个负责任的开发，有时间去操作 postman，不如去写一个脚本。把 postman 的参数都存在一个脚本里，自动去跑，这就是业界的终极方案，即自动化测试。

第一次写脚本虽然多花一点时间，以后再修改，成本就非常低了。不管改任何老代码，只要测试跑挂了，立马就能发现，实现了中等成本、高质量的方案。有人可能会问，中等成本到底有多少成本？根据国内某家大厂的调研，大概要多花 20%~40% 的开发成本。

技术管理的同学听到这里，可能还是觉得行不通，我们公司业务非常忙，如果再增加 40% 的开发成本，直接就要破产倒闭了。实际上不一定，公司总成本可能会下降。

有本书叫做《有效的单元测试》，里面做了一个统计，假设 A 公司不写单元测试，就像咱们国内大部分团队的情况：

• 写代码花费 7 天，然后丢给测试同学；
• 测试发现很多 bug，打回修改，又花了 12 天；
• 26 天后终于上线了，客户还发现了 71 个 bug……

而另一种情况，B 公司写单测试，虽然写代码时间翻倍，花了 14 天，但提交之后基本没 bug，最后上线总时间 23 天，上线之后客户发现的 bug 还很少。

如果这是同一个行业的两家公司，就意味着 B 公司把钱砸在了开发人员上面，提升了开发成本，降低测试成本，最后公司的迭代速度、交付速度变快了，提高了公司的市场竞争力，而且客户满意度更高了；而 A 公司 bug 多，交付慢，竞争力越来越低。

有的开发同学从来没有学过单元测试，抱有一个偏见，觉得自己测自己的代码，肯定不仔细，还是丢给别人测比较好 ，其实这是个误解。

比如左边这个代码，有三种情况，右边只写了一种情况来验证它，不要紧，只要执行一下 mwnw verify，它就会告诉你：你的覆盖率测试覆盖率只有 33%。

写单元测试的核心指标就是覆盖率。那么，做到多少才算优秀？可以看下 GitLab 项目本身，后端 Ruby 做到了98%的单元测试，前端覆盖率做到了 76%。

因为后端都是数据和接口，非常便于做自动化测试，做到 90% 以上才算比较好。前端有的界面难以去自动化测试，可能需要一部分测试同学人工进行验收测试。

Google 是在 20 年前就开始这么做了，微软大概是在 2010 年左右，开始推行开发人员写单元测试，国内某家大厂在前两年开始要求开发人员写单元测试覆盖率达到 80%，否则就不允许合并。

强制测试覆盖率

如果覆盖率不到 80%，怎么去拦截？用 Java 插件 jacoco 就可以了。

比如这个 Java 项目，在 pom 插件里引入 Java 插件，里面写上 limit 0.8，就是最低 0.8 的覆盖率，然后去执行最下面执行命令，就可以算出来测试覆盖率。假设覆盖率只有 0.33，就会产生报错拦截。这个拦截配置在 CI 流水线里设置就可以了，非常简单。

覆盖率下降拦截

但是这样对老项目非常不友好，大量的老项目测试覆盖率是 0。如何让它逐渐提高，减少 bug 呢？极狐GitLab 有一个更好的方案：覆盖率下降拦截。

比如老项目的覆盖率 50% 多，而这次提交新写的代码又导致覆盖率下降了 2.85%，这个时候就会被拦截了。同时还会对测试报告进行标准化采集，与上面提到的代码质量报告类似。

合并请求的页面是开发同学最常使用的，极狐GitLab 把很多报告都做在这个页面里，让开发同学自助修复，不需要跳到任何别的地方，大幅度提高开发效率。

单元测试覆盖率报表

评估一家公司的技术水平，如果只看一个指标，就看单元测试覆盖率，如果看两个指标，则再看代码规范的错误率，即代码不规范的比例（代码规范错误率报表功能即将上线，敬请期待）。

极狐GitLab 会自动生成一个单元测试覆盖率报表，技术管理者可以看到公司所有项目的覆盖率排行榜。这样就知道各个团队的代码质量如何，bug 多不多。

通过报表里的走势图可以一目了然看到，项目覆盖率是不是在平滑上升。因此 Code Review 不需要去评判是否把老接口改挂了，假设改了一个地方，可能被几十个地方调用，人脑不是编译器，判断不出来，只有自动化测试才能够解决这个问题。

哪些需要人工审核？

Code Review：英文术语

前面提到的这么多问题都通过持续集成自动解决了，我们还需要人工审核吗？答案是要的。

比如 Java 项目， spring 项目里面提交了一个 SQL 文件，这段代码应该提交到代码库里进行数据库评审。那么当你新做了一个业务逻辑，建了一个表或者加了一个字段，怎么去评判数据库设计合不合理、文档放在哪里？要不要开个评审会？

直接把它提交到 Git 里，走代码评审就好了，这是非常成熟的一个流程。

如上图，用户表里有个字段：积分，怎么翻译？很多同学词典一搜积分叫 integral，就贴进数据库建表了，等来了个过了英语四级的新同学，发现这是数学积分，数学术语，跟用户积分完全没关系。

怎么样才能正确翻译？可以通过整句翻译，结合场景等方式更加准确，比如积分可以换礼物，才知道积分原来是 point，很多生活中的专用术语，很难通过机器直翻获取。

还有个例子，微信的「拍一拍」在英文版里怎么翻译？第一版不好，后面又改了个词，程序员就疯了，数据库字段和代码都写好了。跑了几个月又改掉了，那么代码里到底是跟着改还是不改呢？

所以英文术语设计是 Code Review 很重要的部分，关系到数据库字段设计，因为数据库字段往往会变成变量，变成接口，它一错后面全错了。

怎么降低术语审核难度？理论上术语应该由需求方提供，假设你是做一个航空行业的外包，这个航空行业有什么术语应该甲方来提供是比较合理。

如果不是外包，是自己做的产品，产品经理应该提供英文术语表吗？这个也不太现实，因为大部分产品只做中文版，产品经理只需要中文界面，没有想过英文术语。于是，代码中用什么英文术语，大部分时候都是开发人员自由发挥，一不小心就导致质量的灾难。

设计模式等技巧

Code Review 还要 review 一些业务逻辑、设计模式，即面向对象的最佳实践，有一些编程有更优雅的写法，这个是高级工程师指导初级工程师成长的很重要的一部分。

比如下图左边这段 Java 代码，它的缩进换行都很漂亮，唯一的问题就是看起来这些函数都太简单了，get ID、get name、set ID、set name…

这么简单的代码，全部重复的写，就是体力活，所以业界用 lombok 解决这个问题：定一个 @Setter、@Getter，就可以把这些函数全部省略掉了。

总结

最后我们总结一下，一个完善的持续集成流水线应该是怎样的？它包括：

• 代码规范的质量门禁；
• 强制的单元测试；
• 编译打包；
• 安全扫描。

有了极狐GitLab 这么一个自动落地规范的流程，把研发流程理顺，Code Review 就省心了。

• 极狐GitLab 在持续集成里做了 Git 各种规范的拦截；
• 在极狐GitLab git server 端，做了 Git 规范拦截，包括 commit 规范、分支规范、分支命名规范；
• 在持续集成里做了运行环境的拦截，包括代码扫描、漏洞扫描。

尽量让这些工作自动化，这样高级工程师评审的工作量就大大降低了，初级工程师能够迅速成长，企业可以通过校招实现人才梯队建设，实现降本增效。

🌟 本期极狐 TechTalk《高效 Code Review 秘籍以及Code Review 在实际业务场景中的最佳实践》分享到此完结，希望对你有所启发和帮助！😊