加入收藏 | 设为首页 | 会员中心 | 我要投稿 江门站长网 (https://www.0750zz.com/)- 科技、建站、经验、云计算、5G、大数据,站长网!
当前位置: 首页 > 运营中心 > 网站设计 > 教程 > 正文

我们是怎么做Code Review的

发布时间:2016-10-29 02:43:48 所属栏目:教程 来源:站长网
导读:副标题#e# 前几天看了《Code Review 程序员的寄望与哀伤》,想到我们团队开展Code Review也有2年了,结果还算比较满意,有些经验应该可以和大家一起分享、探讨。 我们为什么要推行Code Review呢?我们当时面临着代码混乱、Bug频出的状况。 当时我觉得要有所

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的

表面上看来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。
熟悉此问题的同事可以更好的保证问题被解决,确保没有引入新问题。
被影响的同事可以及时了解他受到的影响。 

(编辑:江门站长网)

【声明】本站内容均来自网络,其相关言论仅代表作者个人观点,不代表本站立场。若无意侵犯到您的权利,请及时与联系站长删除相关内容!

热点阅读