关山难越,谁悲失路之人;萍水相逢,尽是他乡之客。
百度360必应搜狗淘宝本站头条
当前位置:网站首页 > 编程教程 > 技术文章 > 正文

从零开始 Code Review,实施两年的经验分享

guanshanw 2023-09-17 17:19 21 浏览 0 评论

前几天看了《Code Review 程序员的寄望与哀伤》,想到我们团队开展

Code Review 也有2年了,结果还算比较满意,有些经验应该可以和大家一起分享、探讨。

我们为什么要推行 Code Review 呢?我们当时面临着代码混乱、Bug 频出的状况。

当时我觉得要有所改变,希望能提高产品的代码质量,改善开发团队面临的困境。并且我个人在开发上有很多经验,也希望这些知识能够在团队内传播。

各种考虑后,我们最后认为推行 Code Review 能改善或解决我们面临的很多问题。

这篇文章的目的不是告诉大家怎么在一个团队内推行 Code Review,首先因为我个人仅在一家公司内推行过,并没有很多经验。其次每家公司、每个团队的情况都不太一样,应该根据公司或团队的实际情况选择恰当的方案,并根据成员的反馈来及时调整,推动 Code Review 的实施。

所以,本文是介绍我们公司是如何实施 Code Review 的,我们是如何解决我们遇到的问题的,希望我们的经验能给大家带来些帮助。

行文仓促,如有遗漏或错误,欢迎指正。

流程和规则

经过简单的对比、试用,我们最后采用了 Git Flow+Pull Request(PR)模式来做 Code Review。

Pull Request(PR) 简单的说就是你没有权限往一个特定的仓库或分支提交代码,你请求有权限的人把你提交的代码从你的仓库或分支合并到指定的仓库或分支。

由于 PR 需要有权限的人确认,所以非常适合在这个过程中做 Code Review,是否接受或者拒绝就取决于 Code Review 的结果。

在支持 PR 模式的软件里,每一个 PR 都有一个新增代码的对比(diff)界面。

代码审核者可以在线浏览请求合并的新增代码,并针对有疑问的代码行添加评论,通过这种方式来实现 Code Review。

评论可以被所有有权限查看仓库的人看到,每个人都可以回复任何人的评论,有点像论坛里某个帖子的讨论。

这种模式是事后审核,也就是代码已经提交到了中心仓库,Review 过程中频繁的改动会造成历史签入记录的混乱。

当然 Git 可以采用更改历史记录来解决这个问题,由于容易误操作,我们一般只在基础类库这类要求比较严格的项目上实施。

我们所了解到的支持PR模式的软件都采用 Git 作为源代码版本控制工具,所以我们的源代码版本控制工具也迁移到了 Git。

由于 Git 太灵活了,因此诞生了很多的 Git 流程,用来规范 Git 的使用。

常见的有集中式工作流功能分支工作流Gitflow 工作流Forking 工作流Github 工作流

我们对 Git Flow 做了些调整,调整后的流程被命名为 Baza Flow,定义见后文。

根据 Baza Flow,我们大部分仓库只定义了 2 个主干分支,master 和 develop。(例如,我们有一个仓库有 3 个开发小组同时进行开发,定义了 4 个主干分支,目前还比较顺畅,再多估计主干分支之间的合并就比较繁琐了。)

master 对应生产环境代码,所有面向生产环境的发布来源都是 master 分支的代码。develop 则对应本地测试环境的代码。

绝大多数情况下,QA(测试)只测试 develop 分支和 master 分支的代码。

由于开发人员都在一个团队内,所以我们没有采用基于仓库的 PR,采用的是基于分支的 PR。

我们对主干分支的操作权限做了限制,只有特定的人才能操作,develop 分支是项目开发 Leader 和架构师,master 分支是 QA。

有权限往主干分支合并的成员会按照约定的规则来执行合并,不会合并没有完成审核的 PR。

上面这点其实蛮重要的,所以我们会对有权限合并的人有特别的约定,在什么情况下才能合并代码。(见后文 PR 的说明)

PR 的发起人要主动的推动 PR 的审核,Leader 也会密切关注 PR 审核的进度,在需要的时候及时介入。

我们配置了 CI 服务器只编译特定的分支,通常是 develop 和master 分支。

所有的代码合并到了主干分支之后,都会自动触发编译和本地测试环境的发布,QA 无需依赖开发人员编译的代码来测试,也无需自己手工操作这些,保证了开发人员和测试人员的相互独立。

我们本地测试环境的发布包含了数据库和站点的发布,全自动的,发布完成以后就是一个可用的产品,有时间这部分也可以分享一下。

我们还使用了 Scrum 里面一个很重要的概念:完成定义。

就是我们规定了我们一个任务的完成被定义为:代码编写完成,经过自测,提交的PR经过审核并且合并到主干分支。

也就是说,所有的代码被合并到了主干分支之后任务才算是完成,而被合并到主干分支必须要经过 Code Review,这是强制的。

Baza Flow

当前版本 V0.9

Baza Flow 由 Git Flow 演化而来,Git Flow 的开发模式如下图所示:

由于我们的托管软件对于 Pull Request 的限制,我们对 Git Flow 做了改动,改动的地方有:

  1. 每一个大功能我们会创建一个单独的 feature 分支,项目开发人员基于这个单独的feature 分支创建自己的任务分支。

    比如,对于 CS 2 项目来说,启动的时候分支的创建是:master -> develop -> feature/v2。

    开发人员应该基于这个大特性分支 feature/v2 来创建自己的任务分支,比如创建 XXXX,可以用一个单独的分支 feature/v2-xxxx。

    完成这个任务以后,立即向上游分支(feature/v2)提交 pull request。然后从 feature/v2-xxxx 创建自己的下一个任务分支,比如 YYYY 编辑 feature/v2-yyyy。

    请注意,合并到上游分支的功能必须相对独立而且是可用的,分支任务工作量 0.5-1 个工作日,不宜超过 2 个工作日,超过 2 个工作日不向上游合并,需要向团队解释

    代码经过 Review 以后,可能会进行必要的修改,修改在原分支修改,修改完毕代码合并进上游分支,原分支会定期删除。

    项目组成员在收到合并成功的通知后,请自行从上游大特性分支向下合并到自己当前的开发分支。

    提交 pull request 后创建新任务分支的时候务必知会一下相关配合同事(比如前端的同事),让他们在新的分支上继续开发。

  2. 对于小功能,预计在 0.5-1 个(不超过 2 个)工作日工作量的开发任务,直接基于 develop 分支创建特性分支即可。

  3. 在各个分支遇到的 Bug,请基于该分支创建一个 Bug 分支。

    如果在缺陷跟踪管理系统上有对应的项,命名请使用缺陷跟踪管理系统的ID,比如 BAZABUG-1354 比如这个 Bug 的分支命名就是 bugfix/BAZABUG-1354。

    如果在缺陷跟踪管理系统上没有对应的项,命名请简短的说明修改内容,比如 “JX 9df2b01 引用 bootstrap css 虚拟路径重写,避免出现字体无法找到的问题”,分支命名可以是 bugfix/miss-font。

    完成修改以后提交并推送到中心仓库然后立即向上游分支提交 pull request。

  4. 发起 pull request 以后,请将 pull request 的链接在 IM 上发给代码审核者,以此通知对方及时进行审核。

执行

我们在团队内部提倡质量优先,开发团队不能为了进度牺牲质量,并在团队内部达成了共识。

所以,无论进度有多么紧迫,Code Review 的过程都一定会做。

所有的问题一定会被提出,只是会根据进度的紧迫程度,以及问题的大小,改动成本,决定问题是现在解决,还是加一个 TODO,并记录在缺陷跟踪管理系统内,以防日后遗忘。

多数情况下,我们都会要求立即解决,哪怕因此造成了发布的推迟。

我们深知,其实多数情况下,现在不解决,日后不知道猴年马月才能解决。

我们在团队内推行 Code Review 的过程中没有遇到太多阻力。

原因大概有两点,首先管理层方面了解之前遇到的各种问题,也迫切希望能有所改善,所以从一开始就是支持的态度。

其次,绝大部分开发人员觉得在这个过程中能自己能学习到东西,并没有抵触,遇到很好的意见时大家都还是很高兴的。

最后,慢慢的形成了一种氛围,整个团队都会自觉的维护它。

附一张我们审核的对话图,这位童鞋尝试对系统内部散落各地发业务邮件的代码做一个整理,用一套模式来处理,调整了3版才定调,然后修改了很多细节才通过了合并,前后大概用一个多星期时间:

表面上看来 Code Review 会延缓项目的进度,但是在我们2年多的执行过程中,大多数时候没感觉到有延缓。

原因是,虽然代码合并的周期变长了,但是由于代码质量提高了,导致 Bug 变少了,由于 Bug 引起的返工问题也变少了,因此整体的进度其实并没有延缓。

我个人认为对一个成熟的团队其实做 Code Review 反而会加快整体的项目进度,但是手头上没有统计数据支撑我的观点。

我们每个分支有权限合并的人都不止一个,这样可以保证有人请假不在的时候,代码仍然可以被其他同事审核通过之后合并。

半年前,我们团队加入了很多新成员,刚加入的新同事对规范、项目、产品的熟悉程度都不高,导致了有一段时间,我们遇到了 PR 审核周期变长的问题。

加上之前遇到的一些问题,我们总结了一个说明,目的是减轻 Code Review 对开发人员工作的负担,加快 PR 审核通过的过程。

说明如下:

Pull Request 的说明

任务完成才能提交 PR。

PR应该在一个工作日内被合并或者被拒绝。

PR在有严重问题(包括但不限于架构问题、安全问题、设计问题),太多问题,或者任务无效的情况下会被拒绝。

严禁一个PR里面有多个任务,除非它们是紧密关联的。

PR提交之后只允许针对Review发现问题再次提交代码,除非有充足的理由,严禁在同一个PR中再次提交其它任务的代码。

提交PR时候有一个描述框,内容会自动根据Commit的message合并而成。

切记,如果一次提交的内容包含很多Commit,请不要使用自动生成的描述。

请用简短但是足够说明问题的语言(理想是控制在3句话之内)来描述:

你改动了什么,解决了什么问题,需要代码审查的人留意那些影响比较大的改动。

审核人员邀请原则:

1. 在创建PR时,Reviewers(审核人)一栏里主要填写“必需审核人”。只有这些人审核都通过,才允许合并。

2. 除了“必需审核人”外,还有一些其它审核人,我们可以在Description里做为“邀请审核嘉宾”@进来。

3. 主干分支间的合并,如Develop => Master,或Master => Develop等,则需要把整个团队(开发+QA)都列为“必需审核人”。

必须审核人的列表由团队决定,可能包括以下人选:

  • 团队Leader

  • 前端架构师(如果有前端代码改动) (可以授权)

  • 后端架构师(如果有后端代码改动) (可以授权)

  • 产品架构师

  • 对此PR解决的问题比较熟悉的(之前一直负责这部分业务的同事)

  • 此PR解决的问题对他影响比较大(比如认领的任务依赖此PR的同事)

其它审核人,包括但不限于:

需要知悉此处代码改动的人但又不必非要其审核通过的同事可以从这个PR中学习的同事

可以授权指的是,根据约定,Bug修复之类的改动,或者影响较小的改动,前端架构师和后端架构师可以授权团队内的某个资深开发人员,由这个资深开发人员代表他们进行审核。主干分支之间的合并,大型Feature的合并,前端架构师和后端架构师需要参与。

上述审核人关注的视角不太一样:

团队Leader关注你是否完成了任务,前后端架构师关注是否符合公司统一的架构、风格、质量,产品架构师从整个产品层面来关注这个PR。

熟悉此问题的同事可以更好的保证问题被解决,确保没有引入新问题。

被影响的同事可以及时了解他受到的影响。

团队Leader或者产品架构师如果觉得PR邀请的审核者不足或者过多,必须调整为合适的人员,其它同事可以在评论中建议。

收获

我们团队实施 Code Review 收获不少,总结出来大概有以下几点:

  1. 短期内迅速提高了代码质量。

    原因有几个,大家知道自己的代码会被人审核之后写得会比较认真。

    理论上代码质量是由整个团队内最优秀的那个人决定的。

    大家也能在 Review 的过程中学习到其它同事优秀的编码。


  2. Bug数量迅速减少。

    但是这个我们没有数据统计比较,比较遗憾。

    我和 QA 聊过,他给我的数据是在我们的一个新项目每 2 周一次的大发布,平均只会发现 1~2 个 Bug。

    这点提高了整个团队的幸福感,大家不用经常被火烧眉毛。


  3. 团队成员对项目的熟悉程度会比较均衡。

    新同事通过参与 Code Review 能很快熟悉团队的规范。

    代码不会只有个别人了解、熟悉,Bug 谁都能改,新功能谁都能做。

    对公司来说避免了人员的风险,对个人来说比较轻松(谁都能来帮你),可以选自己喜欢的任务做。


  4. 改善团队的氛围

    Review 的过程中会需要非常多的沟通,多沟通能拉近团队成员的距离。并且无论级别高低,大家的代码都是要经过 Review 的,可以在团队内营造一个平等的氛围。

    每个成员都可以审查别人的代码,这很容易激发他们的积极性。

亮一下我们的数据:

我们从 2014 年 1 月 17 日开始第一个 PR 的提交,到 2016 年 7 月 5 日一共发出了 6944 个 PR,其中 6171 个通过,739 个拒绝。日均 11.85 个 PR,最多的一天提了 55 个 PR。

这些 PR 一共产生了 30040 个评论,平均每个 PR 有 4.32 个评论,最多的一个 PR 有 239 个评论。

参与上述 PR 评论的同事一共有 53 位,平均每位同事发出了 539 个评论,最多的用户发出了 5311 个评论,最少的发了 1 个(刚推行 Code Review 就离职的同事)。

需要说明一下,只有简单的问题会通过评论来提出。比较复杂的,比如涉及到架构、安全等方面的问题,其实都会面对面的沟通,因为这样效率更高。

四、

总结

虽然有合适的工具支持会更容易实施 Code Review,但它本身并不特别依赖具体的工具,所以前文并没有具体指明我们用了什么工具,除了 Git。

原因是基于分支的 PR 流程依赖于大量创建分支,而 Git 创建一个分支非常的简单,所以 PR 模式+ Git 是一个很好的搭配。

我们在切换到 Git 之前,也做 Code Review,采用的是提交代码以后把 commit的 Id 发给相关同事来审查的流程。

审核通过以后会在缺陷跟踪管理系统里面评论,QA 同事没见到审核通过的评论就认为任务没有完成,拒绝进行测试。

虽然没有现在这样直接方便,但是也还是做起来了。

PR 审核的过程中,新加入的团队成员常见的问题是不符合代码规范之类的,其实是可以通过源代码检查工具来解决的,这部分我们一直在计划中,并没有开始实施。

相关推荐

七条简单命令让您玩转Git
七条简单命令让您玩转Git

凭借着出色的协作能力、快速部署效果与代码构建辅助作用,Git已经得到越来越多企业用户的青睐。除了用于开发商业及消费级应用之外,众多科学及政府机构也开始尝试使用这...

2023-10-07 12:14 guanshanw

基本完整的关于Git分支branch的操作
基本完整的关于Git分支branch的操作

Git使用背景项目中要用到dev或者其他分支开发完代码,需要将该分支合并到master的需求操作步骤下面以dev名称为lex为分支名为例来操作一遍客户端操作:...

2023-10-07 12:14 guanshanw

Git 进阶(合并与变基)
Git 进阶(合并与变基)

在Git中整合来自不同分支的修改主要有两种方法:合并(merge)以及变基(rebase)合并(merge)merge流程图merge的原理是找到这两个分...

2023-10-07 12:13 guanshanw

Git学习笔记 003 Git进阶功能 part5 合并(第一部分)

合并(merge)是很常用的操作。尤其是一个庞大的很多人参与开发的企业级应用。一般会设定一个主分支,和多个副分支。在副分支开发完成后,合并到主分支中。始终保持主分支是一个完整的,稳定的最新状态的分支。...

非标题党,三张图帮你理解git merge和git rebase的区别
非标题党,三张图帮你理解git merge和git rebase的区别

初始场景:基于正常的开发分支修改几个小bug,然后在合并到开发分支上。gitmergegitcheckoutfeaturegitmergeho...

2023-10-07 12:13 guanshanw

git 初次使用(01)
git 初次使用(01)

先从github上克隆代码下来:使用vscode克隆代码如下图,填写上github仓库地址:vscode有时候克隆代码速度比较慢,可以用命令行方式克隆gitc...

2023-10-07 12:12 guanshanw

Git 远程操作

4.Git远程操作命令说明gitremote远程版本库操作gitfetch从远程获取版本库gitpull下载远程代码并合并gitpush上传远程代码并合并4.1远程版本库操作gitre...

Git常用命令-总结
Git常用命令-总结

创建git用户$gitconfig--globaluser.name"YourName"$gitconfig--globaluser.em...

2023-10-07 12:12 guanshanw

git中删除从别人clone下来项目的git信息,并修改为自己的分支

如果你从别人的Git存储库中克隆了一个项目,并想要删除与该存储库相关的Git信息,并将其修改为你自己的分支,则可以执行以下步骤:使用gitclone命令克隆存储库:gitclone<u...

git系列-回滚和放弃本地修改

回滚历史提交就是reset的功能。这种情况是已经提交远程仓库,需要回滚到之前的提交。gitreset--hardcommitId//注:强制提交后,当前版本后面的提交版本将会删掉!gi...

GIT使用小技巧大全
GIT使用小技巧大全

在大型软件工程的开发过程中,版本控制是无法绕过去的;目前来说,最火的版本控制软件就是GIT了。早两年SVN比较火,不过被大神linus喷了几次后,就日落西山了,...

2023-10-07 12:11 guanshanw

git相关命令-上
git相关命令-上

这些命令都是看了文档后,个人觉得比较有用的一些,展示给大家。回到远程仓库的状态抛弃本地所有的修改,回到远程仓库的状态。gitfetch--all&...

2023-10-07 12:10 guanshanw

Git命令行接口:掌握Git的必备技能
Git命令行接口:掌握Git的必备技能

Git是一款强大的分布式版本控制工具,它支持命令行界面操作。熟练掌握Git命令行接口,是开发者使用Git的必备技能之一。在这篇文章中,我们将介绍Git命令行接口...

2023-10-07 12:10 guanshanw

Git命令详解
Git命令详解

相信各位小伙伴们应该都对git有一些了解,毕竟作为代码管理的神器,就算不是IT行业的小伙伴肯定也或多或少的听说过一些。今天就来和小伙伴们分享一下自己总结的常用命...

2023-10-07 12:10 guanshanw

工作7年收集到的git命令
工作7年收集到的git命令

概念git中的术语解释:仓库也叫版本库(repository)stage:暂存区,add后会存到暂存区,commit后提交到版本库git安装linux...

2023-10-07 12:10 guanshanw

取消回复欢迎 发表评论: