💡 近日,在极狐 TechTalk 播上,极狐 (GitLab) 高级解决方案架构师杨周 分享了高效 Code Review 秘籍以及 Code Review 在理论业务场景中的最佳实际,并手把手带大家基于极狐 GitLab 进行了 Code Review。
以下内容整顿自本次分享,干货爆满,分为高低两期:
- 上期回顾👉代码品质问题、分支创立与评审、Git 标准、安全漏洞;
- 这期持续剖析👉代码标准、Code Review 实际中大家最关注的几个问题。
关注「极狐 GitLab」不迷路,一起 get Code Review 小技巧吧!
一、代码标准
1. 常见的代码标准问题有哪些?
1.1 不同公司有不同代码标准
很多公司制订了本人的代码标准,甚至是打印进去贴在墙上。Code Review 标准到底要不要去人工干预?
首先,咱们看一下这段 Java 代码,这里有 3 个问题,Java 书写标准个别要求:
- 类名和括号应该在同一行;
- 函数名应该用驼峰;
- 缩进应该用空格,而不是 tab。
1.2 不同语言的标准各不相同
咱们再来看一个 Go 的代码。很多同学可能要抢答了,一眼看到这里的数据不对,底下俩空格下面是 tab?十分遗憾,这其实是对的。
大部分语言都是空格缩进,但 Go 官网要求用 tab……go fmt hello.go 会强制转换。这也是十分新鲜的做法,就是语言官网自带标准,所以大家不要带着一个语言的习惯去评审另一个语言。
1.3 魔法数字难以保护
最初来看这个 PHP 代码有什么问题。这里有一个大括号换行问题,上面又有一个换行。
其实这些都是对的。PHP 很风行的标准是 PSR-2,它定义的是类前面大括号必须换行,和 Java 恰恰相反,但 if else 不换行。
大家要留神一下,不要搞得精神分裂了。
所以,这段代码超级换行没有任何问题,惟一的问题是魔法数字。if else 应用了 1 和 2 难以了解,这里应该用常量来示意。还有它的 if else 嵌套的太深了,复杂度太高,难以了解,难以保护。
代码标准有很多种问题,比方每个人的书写格调和习惯不同,人工去查看缩进换行节约大量工夫,更别说老我的项目,有很多的问题导致代码标准无奈落地,这些都是业界常见的问题。
2. 极狐 GitLab 是怎么解决这些问题的?
首先,咱们举荐大家采纳业界出名的代码标准,而不要轻易本人造标准。因为除了写公司的代码之外,咱们可能还会加入很多开源我的项目。开源我的项目个别都采纳业界出名的标准,如果你本人造了一个跟大家都不一样的标准,做别的我的项目会比拟麻烦。
上图是几家出名标准,比方 Java 有 Google 标准、PMD 标准,Go 语言有官网标准,PHP 有 PSR 标准。
这些标准次要分成两大类:格调标准和复杂度标准。
- 格调标准规定缩进、换行、空格这些标准;
- PMD 限度的是复杂度,复杂度不能大于 10,这是默认值,如果团队目前复杂度比拟高,能够本人去批改。另外,行数不能超过 60 行,否则太简单了;类不能超过 1000 行,否则违反了面向对象的繁多职责。
个别举荐大家先落地格调标准,等所有成员都上手了,再落地复杂度标准,再逐步收紧,比方先调成 15 复杂度,逐步收到 10 以下,甚至收到 5 以下。
业界出名代码标准都有一个特点,全副是有配套的命令行工具,而不是一个文档。有的大厂有本人的标准,收回 PDF 给大家看,这样是不适合的,而应该是一个配置文件加命令行工具,能让大家一键执行,才是一个合格的标准。比方 Java Checkstyle、Code Sniffer、前端 eslint,连 Markdown 文档也是有标准的,比方阮一峰标准,用 Lint-md 来扫描。
3. 怎么在团队里落地标准?
咱们以 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 提供了一个更好的形式。
4. Java 代码标准和极狐 GitLab MR 买通
同样以 Java 标准为例,咱们先引入 checkstyle 插件,下载代码标准,须要申明查看标准容许失败,失败后要把报告采集上来,并生成一个代码标准的报告,但它不是极狐 GitLab 的格局,还须要通过 Maven validate 对格局进行转换。
采集上来之后,在合并申请里就能看到多进去一个叫做“代码品质模块”的区域,这样开发同学一提交代码就能看到代码有标准问题,就能够本人修复。
上图能够看到历史遗留问题有几千个,大家暂停手头工作一起来修也不事实,现实的形式是做成增量代码标准,极狐 GitLab 曾经实现了增量代码标准报告。
5. 极狐 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%,否则就不容许合并。
1. 强制测试覆盖率
如果覆盖率不到 80%,怎么去拦挡?用 Java 插件 jacoco 就能够了。
比方这个 Java 我的项目,在 pom 插件里引入 Java 插件,外面写上 limit 0.8,就是最低 0.8 的覆盖率,而后去执行最上面执行命令,就能够算进去测试覆盖率。假如覆盖率只有 0.33,就会产生报错拦挡。这个拦挡配置在 CI 流水线里设置就能够了,非常简单。
2. 覆盖率降落拦挡
然而这样对老我的项目十分不敌对,大量的老我的项目测试覆盖率是 0。如何让它逐步进步,缩小 bug 呢?极狐 GitLab 有一个更好的计划:覆盖率降落拦挡。
比方老我的项目的覆盖率 50% 多,而这次提交新写的代码又导致覆盖率降落了 2.85%,这个时候就会被拦挡了。同时还会对测试报告进行标准化采集,与下面提到的代码品质报告相似。
合并申请的页面是开发同学最常应用的,极狐 GitLab 把很多报告都做在这个页面里,让开发同学自助修复,不须要跳到任何别的中央,大幅度提高开发效率。
3. 单元测试覆盖率报表
评估一家公司的技术水平,如果只看一个指标,就看单元测试覆盖率,如果看两个指标,则再看代码标准的错误率,即代码不标准的比例(代码标准错误率报表性能行将上线,敬请期待)。
极狐 GitLab 会主动生成一个单元测试覆盖率报表,技术管理者能够看到公司所有我的项目的覆盖率排行榜。这样就晓得各个团队的代码品质如何,bug 多不多。
通过报表里的走势图能够高深莫测看到,我的项目覆盖率是不是在平滑回升。因而 Code Review 不须要去评判是否把老接口改挂了,假如改了一个中央,可能被几十个中央调用,人脑不是编译器,判断不进去,只有自动化测试才可能解决这个问题。
三、哪些须要人工审核?
1. Code Review:英文术语
后面提到的这么多问题都通过继续集成主动解决了,咱们还须要人工审核吗?答案是要的。
比方 Java 我的项目,spring 我的项目外面提交了一个 SQL 文件,这段代码应该提交到代码库里进行数据库评审。那么当你新做了一个业务逻辑,建了一个表或者加了一个字段,怎么去评判数据库设计合不合理、文档放在哪里?要不要开个评审会?
间接把它提交到 Git 里,走代码评审就好了,这是十分成熟的一个流程。
如上图,用户表里有个字段:积分,怎么翻译?很多同学词典一搜积分叫 integral,就贴进数据库建表了,等来了个过了英语四级的新同学,发现这是数学积分,数学术语,跟用户积分齐全没关系。
怎么样能力正确翻译?能够通过整句翻译,联合场景等形式更加精确,比方积分能够换礼物,才晓得积分原来是 point,很多生存中的专用术语,很难通过机器直翻获取。
还有个例子,微信的「拍一拍」在英文版里怎么翻译?第一版不好,前面又改了个词,程序员就疯了,数据库字段和代码都写好了。跑了几个月又改掉了,那么代码里到底是跟着改还是不改呢?
所以英文术语设计是 Code Review 很重要的局部,关系到数据库字段设计,因为数据库字段往往会变成变量,变成接口,它一错前面全错了。
怎么升高术语审核难度?实践上术语应该由需求方提供,假如你是做一个航空行业的外包,这个航空行业有什么术语应该甲方来提供是比拟正当。
如果不是外包,是本人做的产品,产品经理应该提供英文术语表吗?这个也不太事实,因为大部分产品只做中文版,产品经理只须要中文界面,没有想过英文术语。于是,代码中用什么英文术语,大部分时候都是开发人员自由发挥,一不小心就导致品质的劫难。
2. 设计模式等技巧
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 在理论业务场景中的最佳实际》分享到此完结,心愿对你有所启发和帮忙!😊
欢送关注极狐 GitLab,获取更多干货常识,开启你的效力晋升之旅!