关于java:从零开始-Code-Review两年实战经验分享

作者:wenhx
http://www.cnblogs.com/wenhx/…

前几天看了《Code Review 程序员的寄望与哀伤》,想到咱们团队发展 Code Review 也有2年了,后果还算比较满意,有些教训应该能够和大家一起分享、探讨。

咱们为什么要推广Code Review呢?咱们过后面临着代码凌乱、Bug频出的情况。

过后我感觉要有所扭转,心愿能进步产品的代码品质,改善开发团队面临的窘境。并且我集体在开发上有很多教训,也心愿这些常识可能在团队内流传。

各种思考后,咱们最初认为推广Code Review能改善或解决咱们面临的很多问题。

这篇文章的目标不是通知大家怎么在一个团队内推广Code Review,首先因为我集体仅在一家公司内推广过,并没有很多教训。

其次每家公司、每个团队的状况都不太一样,应该依据公司或团队的理论状况抉择失当的计划,并依据成员的反馈来及时调整,推动Code Review的施行。

所以,本文是介绍咱们公司是如何施行Code Review的,咱们是如何解决咱们遇到的问题的,心愿咱们的教训能给大家带来些帮忙。

行文仓促,如有脱漏或谬误,欢送斧正。关注Java技术栈微信公众号,在后盾回复关键字:Java,能够获取更多栈长整顿的Java技术干货。

一、流程和规定

通过简略的比照、试用,咱们最初采纳了Git Flow+Pull Request(PR)模式来做Code Review。(PR模式详情可参见 Git工作流指南:Pull Request工作流)

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

因为PR须要有权限的人确认,所以非常适合在这个过程中做Code Review,是否承受或者回绝就取决于Code Review的后果。

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

代码审核者能够在线浏览申请合并的新增代码,并针对有疑难的代码行增加评论,通过这种形式来实现Code Review。

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

这种模式是预先审核,也就是代码曾经提交到了核心仓库,Review过程中频繁的改变会造成历史签入记录的凌乱。

当然Git能够采纳更改历史记录来解决这个问题,因为容易误操作,咱们个别只在根底类库这类要求比拟严格的我的项目上施行。

咱们所理解到的反对PR模式的软件都采纳Git作为源代码版本控制工具,所以咱们的源代码版本控制工具也迁徙到了Git。关注Java技术栈微信公众号,在后盾回复关键字: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服务器(什么是CI)只编译特定的分支,通常是develop和master分支。

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

咱们本地测试环境的公布蕴含了数据库和站点的公布,全自动的,公布实现当前就是一个可用的产品,有工夫这部分也能够分享一下。

咱们还应用了Scrum外面一个很重要的概念:实现定义。

就是咱们规定了咱们一个工作的实现被定义为:代码编写实现,通过自测,提交的PR通过审核并且合并到骨干分支。

也就是说,所有的代码被合并到了骨干分支之后工作才算是实现,而被合并到骨干分支必须要通过Code Review,这是强制的。

因为咱们的托管软件对于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审核的过程中,新退出的团队成员常见的问题是不合乎代码标准之类的,其实是能够通过源代码查看工具来解决的,这部分咱们始终在打算中(( ╯□╰ )),并没有开始施行。

近期热文举荐:

1.600+ 道 Java面试题及答案整顿(2021最新版)

2.终于靠开源我的项目弄到 IntelliJ IDEA 激活码了,真香!

3.阿里 Mock 工具正式开源,干掉市面上所有 Mock 工具!

4.Spring Cloud 2020.0.0 正式公布,全新颠覆性版本!

5.《Java开发手册(嵩山版)》最新公布,速速下载!

感觉不错,别忘了顺手点赞+转发哦!

评论

发表回复

您的邮箱地址不会被公开。 必填项已用 * 标注

这个站点使用 Akismet 来减少垃圾评论。了解你的评论数据如何被处理