素材巴巴 > 程序开发 >

万字分享:以Code Review 最佳实践,解答降本增效 or 增加成本之问(上)

程序开发 2023-09-22 17:24:05

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

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

关注「极狐GitLab」不迷路,一起 get Code Review 小技巧吧!

前言

在软件行业,Code Review 一直被认为是一种非常好的工程实践,但在实际工作中,却三令五申也执行不下去。或是业务跑太快,没时间 Code Review,或是即使发起 Code Review ,但 Reviewer 秒同意,收效甚微……

其实,代码里有很多问题是值得去评审的,工程师自己可能会忽视。比如 Git 规范、代码规范、是否影响老代码、数据库英文术语设计、设计模式,业务逻辑是否正确等等。

不只是初级工程师在代码评审过程取得进步,高级工程师之间互相评审,也往往能发现一些问题,团队成员都可以互相学习和成长。

常见的代码质量问题有哪些?


随意提交

下图是网上曝光的一个大厂的代码,搞笑但真实。有人在注释里面恶搞,后面的同学看到接着恶搞。当然它也没有什么危害,但这说明代码没有人管,大家都可以随便提交,长此以往,代码问题就会逐渐的爆发。

无用文件

有的用户反映 Git 越来越慢,经过排查发现,有人提交了 jar 包,比如第三方包 rabbitmq,这是初学者常犯的错误。

其实依赖包不应该提交代码库,应该通过 maven 网络仓库安装。有人问:不少国产包没有发布到 maven 开源仓库,只提供 jar 包下载,比如云支付,怎么办?好办,发布到极狐GitLab 私有 maven 仓库即可。

 魔法数字

大家在老项目里可能都见过 1、2、3、4 这种写法,后面接手的人完全看不懂,甚至自己过一段时间也忘了。

有个说法叫「优秀的代码都是不用注释的」,这句话是有道理的。如果再给左边的代码加注释,需要在 1234 的地方都加,反而累赘。正确做法是用英文常量来表示数字,从而避免这种魔法数字问题。

以上问题最基础的解法就是 Code Review。

创建分支和选择评审


创建分支的 2 种方式

代码评审的基础是把主干锁定起来,很多人可能会说:“我知道,我们都把主干分支锁定了,大家都在开发分支上写代码,写完了直接提交,最后开发分支被直接合并主干分支,就上线了。” 但这样的话,保护机制就形同虚设了。

保护分支的核心理念是保护所有公共分支,如 develop ,是不可以直接推送的,如果有人尝试推送会直接报错。保护分支以后,就需要创建临时分支。

怎么创建临时分支才是正规的、高效的?有几种方式。

方式一:本地创建分支,网页创建合并请求

最常见的方法是:

1. 在本地拉分支 git checkout -b;

2. 起一个贴切的、不重复的分支名字,这让不少开发同学头疼,花费了不少时间思索;

3. 推送完了以后会提示打开网页,创建一个合并请求 MR。

这里有三步操作,打开了网页合并请求之后,还要再点一下,效率是比较低下的。怎么优化?极狐GitLab 提供了一个非常简洁的方式。

方式二:先创建需求,一键拉分支

首先,创建一个需求或者 bug,然后就可以一键拉分支了。

分支名字自动起好了,用需求 ID 开头,来确保它的全局唯一性,后面跟着需求标题,英文保留,那汉字怎么办?GitLab 以前是直接丢弃了汉字,作为GitLab 的中国发行版——极狐GitLab 自动把汉字转成拼音,更适合中国开发者。

创建好分支以后,就需要选择同事进行评审,这也有好几种方式:

选择评审的 3 种方式

方式一:每次创建 MR 时选人

极狐GitLab 免费版可选 1 个人,专业版可选多个人。

注意:这里有个草稿功能,草稿不需要审核,也不能合并,前文说的需求一键拉分支,就会自动加上草稿前缀,非常方便。

但是,每次提代码的时候都选人去审比较低效,到底选几个人合适?选中的人有没有空?这是未知的。因此,极狐GitLab 提供了一个更高效的方式——提前配置小组评审,这样就不用每次选人了,也不用担心选的人请假了。

方式二:一次设置,多个小组评审

以后端项目为例,应该由高级后端小组评审,可以设置需要几个人审,比如金融合规要求至少 2 个人,其他行业为了节约成本往往设置为 1 个人。

还可以设置多个小组评审,比如安全团队也要审核是否有安全漏洞,这是极狐GitLab 的特色功能,业界很多同类产品不具备该功能。

方式三:代码所有者( Code Owners)

极客同学喜欢这种 Configuration as code(配置即代码)方式。比如这个目录应该谁负责审,把他的名字写上,这里可以是多个人,然后在网页配置里可以看到所有符合条件的用户。

接下来我们聊聊开发团队非常需要的 Git 规范。

Git 规范


首先要不要在 Code Review 的时候审核 Git 规范?

来看一个真实代码:李雷同学提交了一个分支想要合并,我们看到直接就警告⚠️了,这里提示他改了 6000 多行代码,100 多个文件,网页都显示不下了。

这是个常见的错误——用自己的名字做分支。人名用作分支的确和别人不冲突,但在一个分支里提交好多个需求,然后请同事评审,同事就疯掉了。

那怎么做才是正规的呢?

JiHu Flow 

 业界的最佳实践是:每个需求一个分支,这也是 JiHu Flow 推崇的做法。严格来说,是每个议题一个分支,议题包括:需求、bug 以及重构任务等各种类型。

那么主干就没有用了吗?

不是的,主干用来修复线上的紧急 bug。比如线上 1.2.0 这个版本,有紧急 bug,不可能等下一个迭代去修,那就拉起一个临时分支——Hotfix,修完之后打一个新的 Tag——v1.2.1。这个三位数字的版本号也是有规范的,叫做语义化版本号规范。按照这个规范,当新的迭代想发布时,合并过来需要打一个新的 Tag——v1.3.0,这里的逻辑是:有新功能应该增加数字中间这一位,修 bug 应该加最后一位。

分支命名规范

怎么能够确保大家的分支拉取命名都是规范的呢?靠口头传达?靠入职培训?分支命名规范不可能人工评审,因为分支先推送到服务器,才能评审,为时已晚。

所以分支命名规范应该做成 Git 服务器的钩子功能,只要不规范,就在入库之前进行拦截。极狐GitLab 提供了这个功能,可以公司统一配置,也可以在每个项目里配置,它是一个正则表达式,可以约束各种分支命名规范。

我们看上图这段代码,有人拉了一个分支叫 login,难以看出是需求还是 bug,他没有在网页上按照规范去创建(网页上会自动按照规范去创建,不需要人去干预),自己手动拉的话就会很难遵守这个规范。当他 push 的时候,直接就会报错,根本推送不上来。

我们再看看上图这个代码有没有别的问题?可以看到下面有一个 PDF,非常奇怪,PDF 是二进制文件,为什么要提交 Git?可能是要放在公司的网盘里,就大家看一看就好了。

那怎么拦截?很多同学对 Git 比较熟,知道用 gitignore,但它只能拦截已知的文件,拦了PDF,又有人提交了word,或者MP4视频……防不胜防。

怎么拦截这种无穷无尽的后缀?极狐GitLab提供了一个高级玩法,拦截文件大小就可以了。

杜绝提交垃圾文件

比如可以拦截任何大于 1M 的文件,大于 1M 的话,十有八九它不是代码。如果是一个大的游戏素材,的确需要提交的库里,那怎么办呢?可以用 git lfs (大文件存储),这里就不展开说了。

也可以配一些常见的文件名,比如 Jar 包这些,它与 gitignore 的区别是可以在整个公司群组级别设置,对所有新项目生效,这样即使新项目开发组长没有配 gitignore 也不要紧。

合并请求:git log

我们来看看下面这个代码合并页面有没有什么问题。

可以看到提交信息非常的诡异,2.2.0 版本发布了两次,修改了一个 readme 文档,修了一个 bug,做了一个需求,但完全看不出来修了什么 bug。

这是小白常犯的错误,正确的写法应该是什么样呢?

下图是知名的开源项目 angular,前端同学可能比较熟,它的写法就很漂亮,比如:docs 说明改了什么文档,fix 说明修了什么bug。

这些规范如何学习?如何在团队落地呢?

git cz 取代 git commit

学习好办。大家在本地安装 git cz 命令,配置 angular 规范来取代 git commit,就会弹出来 10 个英文前缀供选择。但这个方法是大家本地学习用的,不可强求,有得人本地电脑就没有安装。

那么作为开发组长、研发管理人员,应该怎么在团队内落地规范呢?

检查 commit message

在极狐GitLab 的仓库配置里,可以配置 commit message 规范正则表达式。我们约定 10 来个前缀,后面可以写汉字,如果团队英文水平都很好的话,也可以要求写 a 到 z。这样如果有人瞎提交,一推送就会失败。

代码关联需求

即使我们严格按照刚才说的格式去写,比如说 build: server env,写的时候知道什么意思,过了一段时间就记不得了,别人看着也有点晕,这里到底是做了什么需求?

因此,更好的方式是把需求 ID 写上,叫做代码关联需求,如下图效果,“#1” 就会变成一个链接,点过去就是这个需求。

提交的时候也不用写很长,一个需求可能很复杂,我们很难去描述它,有 ID 非常方便了,这个配置也是在 commit message 规范里,大家配一下 “#d+” 就可以了。

Git规范总结

GitLab 最初诞生的时候,就做了 Git 仓库功能,所以在 Git 规范方面非常成熟,包括 commit message、规范的正则表达式、垃圾文件后缀拦截、文件大小拦截,分支命名规范等常用的功能,这些都属于 Git Workflow。

在 Code Review 过程中,如果配好了极狐GitLab,就帮你解决了大部分规范落地的问题,完全不需要人去干预,节省了高级工程师的时间。

安全漏洞


代码重复

举个🌰:有一天,李雷提交了 pom.xml,做了新业务,引入了一个 log4j 2.14.1。评审的同学质疑 log4j 旧版本好像有漏洞,那么李雷就需要去查看哪个版本没有漏洞。这时候李雷就非常的郁闷,引了那么多包,哪个包有漏洞谁知道?靠人工去核实每个包效率也太低了。

当时 Apache Log4j 漏洞大爆发就产生了很大的危害,因为开发同学不可能投入很多精力去盯着哪个漏洞又爆发了,所以 Apache 当时紧急修复了好几个版本,新版本发出来又被发现有漏洞,大家都非常抓狂。

开源漏洞库 

这几年安全问题越来越严峻,各种漏洞层出不穷,于是 GitLab 做了一个开源漏洞库,有专职的安全人员去维护它,全球开发者也一起参与,做成了一个开源的 Git,这也是极狐GitLab 的技术理念,叫做一切皆代码。我们倾向于把一切的配置都存到 Git 仓库里,一方面便于用户开源协作,另一方面 Git 是一个很好的技术底座,可以和各种工具打通,如果存成别的格式,就很难打通了。

有了漏洞库以后,怎么执行?

很简单,先把漏洞库克隆下来,放在一个本地目录里,然后执行一下就好了。我们把这些安全工具都封装成了 docker,执行起来非常简单,并且可以离线合规运行。

像金融等行业要求各种工具和代码扫描都必须在内网里运行,极狐GitLab 是完全支持的,可以定时在联网的环境下拉取漏洞库,再转移到内网里,来确保安全合规。

当然上面说的是原理,实际上我们不是在本地敲命令,而是希望安全漏洞能够自动被拦截,不需要人工去做任何操作,那么怎么来配置?

离线运行,安全合规 

上图是一个GitLab CI 的配置, 就是一个 yml 文件。这是一个 Java 项目,里面就执行了一个 build 命令,收集了编译的 Jar 包。想要在里面引入安全扫描,只要两行代码:

CI 官方插件:一键配置安全扫描 

这就是极狐GitLab 的插件机制,yml 里面可能有几百上千行,大家也可以在开源代码找到它,去学习它的写法,并且可以做自己的插件,给自己的项目用,给整个公司用,甚至开源出去给大家用。

写了这一行之后,它就会在我们提交代码合并时进行安全扫描拦截,MR 页面中间出现一个模块,叫做安全扫描,发现了 4 个漏洞——Apache log4j,不允许合并。

每个漏洞都可以点开,比如这里提示要把这个包升级到某某版本,就消灭漏洞了。解决方案就来自于 GitLab 漏洞库。

以前,想解决安全漏洞问题,需要安全人员来去研究漏洞库,选型安全工具。当开发同学的代码,经过 Code Review 之后合并上来,部署到测试环境,安全人员上去扫描,有问题就打回修改以后再提交。

这个跨部门的反馈周期是很长的,怎么提高效率?

安全左移

我们可以把安全工具左移,放到持续集成里,自动下载漏洞库跑起来,这样当开发人员提交代码的时候,有问题的直接打回自助修改。这个就是业界的安全左移理念,叫做 DevSecOps。

极狐GitLab 的依赖扫描支持各种语言,如 Go、PHP、 Java、NPM。

极狐GitLab 还提供九大安全扫描能力,除了依赖扫描之外,还有静态扫描、动态扫描,以及容器扫描比如 Docker 等。现在很多公司现在已经用上 Docker 了,还有很多正在做容器化。Docker 里面一不小心引入的基础镜像里,就可能有很多漏洞,肉眼是看不出来的,需要扫描工具去发现。

极狐GitLab 安全漏洞防护,依靠流水线的自动化,让开发同学自主修复,从而提高研发效率。在 Code Review 的时候,弹出来并自助修复。

🌟 《高效 Code Review 秘籍以及 Code Review 在实际业务场景中的最佳实践》上期就分享到这里,希望对你有所启发和帮助 😊

下期精彩,敬请期待!


标签:

上一篇: Maven环境构建及使用 下一篇:
素材巴巴 Copyright © 2013-2021 http://www.sucaibaba.com/. Some Rights Reserved. 备案号:备案中。