Nicky's Blog

你是怎样提交Code Review的?

人人都会提代码review,怎样才是比较好的适合团队的方式呢?

关于revision

相信很多人以前跟我一样,是这样提交的:
比如:一开始同学A在branchA修改提交了部分代码diff1(revision:1)

1
2
3
4
String a = "abc" + "bcd" + "cccc"
String b = test;
String c = b.getBytes();
.....

提交到reviewboard给大家review的时候:

同学B率先发现了空指针隐患,反馈给同学A,同学A马上修改代码提交为diff2:(revision2)

1
2
3
4
5
6
String a = "abc" + "bcd" + "cccc"
if (test != null) {
String b = test;
String c = b.getBytes();
.....
}

此时同学C有空了,也准备review同学A的需求代码,他就郁闷了,我是看diff1呢,还是看diff2即可? 一脸懵~

如果在同学C之前已经有多次的修改呢? 就要把所有diff一遍遍过,但此时这些都是无用的产物,对于新的review同学,只需要关注你最后一次完整提交即可,不管你之前的版本

所有,正确的发送review方式,应该是:

  1. 保证每一次提交diff都是全量的,比如你基础revision是1, 下一次就要 1:x, 而不是x
  2. 其他同学可在中途介入来查看你的信息,而不用关注全部,他们可以通过查看两个diff文件之间来查看你的修改差异,reviewboard提供了 diff between功能

关于文件类型

一般情况,我们不建议把图片类型的或者二进制文件,提交到reviewboard,因为这些文件都无法正常显示,大家也无法帮忙review,私下自己记得提交即可,否则一次功能,会发现一堆diff文件,其实java文件就几个,会对reviewer造成视觉或者心理影响. 但是这个不是必须的,看大家共识而定.

关于描述

review提交的时候,一般是有标题\简介部分
标题: [Feature] featureid: your feature name 功能开发类
[Bugfix] bugid: xxxxx bugfix类
[Optimized] 代码优化重构类

简介部分: 必须描述功能主要功能\ 1. 2. 3. 描述清楚,并且附上产品需求文档,必要的时候,写明测试通过某些部分等,这些在对于reviewer来说至关重要,因为他们的时间非常宝贵,他们很多是不熟悉你的需求文档的,不期望二次沟通来获取到他们关注的信息,所有发出去的时候,需要把必要的信息暴露出去,很多时候,也需要面对面review,一遍描述,一遍看diff.

总结

  1. 每一次review提交都是全量diff
  2. 描述清楚review提交的内容信息,减少不必要的沟通成本

转载声明: 本文转载前需与作者联系并标明文章出处