使⽤PhabricatorArcanist进⾏CodeReview的流程
使⽤ Phabricator & Arcanist 进⾏ Code Review 的流程
之前我们讲过,这次我们需要在 git 基础上加⼊ code review 机制。
下⾯ git 命令的简写,请参考上⾯链接中的内容。
Before Using Phabricator & Arcanist
Minor Change
如果只是修改⼩部分代码,不需要开 feature branch,⽐如修复某个崩溃,开发⼯作的流程为:
1. git co develop; git pull
2. 修改代码
3. git commit -a -m “xxxx”
4. 重复2和3
5. git pull –reba,如果有冲突,解决冲突
6. git push
7. 重复1到6
Feature Development
如果进⾏⼤改动,需要开 feature branch,⽐如业务改造,开发流程会多增加⼏步:
1. git co develop; git pull
2. git co -b new_feature_branch; git push -u origin new_feature_branch
3. 修改代码
4. git commit -a -m “xxxx”
5. 重复3和4
6. git pull –reba,如果有冲突,解决冲突
7. git push
8. 重复上⾯3~7步
9.药方子
git co develop; git merge –no-ff new_feature_branch
10. git branch -d new_feature_branch
11. 重复1到10
也就是说,在 new_feature_branch 开发完成前,修改都是在 new_feature_branch 上进⾏ commit,最后new feature 完成后,再merge 到 develop。
After Using Phabricator & Arcanist
,描述了⼀次 arc diff 以及 arc land 的过程。
phabricator的code review流程,⽐较灵活,但需要掌握的配置也多,这⾥是它的。
code review ⼤致的流程为:
1. 修改代码
2. arc diff,⽣成 revision,它的状态为 Need Review
3. 邮件通知 reviewer 进⾏审核。如果审核不过,它会被置为 Need Revision,再次 diff 后,会重新回到 Need Review 状态;如果审核通过,它会被置为 Accepted
4. arc land 后,revision 的状态会变为 Clod
真实的开发情况会⽐上⾯描述的流程复杂⼀些,⽐如我们不会等待⼀次 code review request 被Accepted 后,才继续开发,⽽是等待过程中继续开发。
为了适应真实的开发情况,制定了⼀个流程和规范。
先说⼀下流程图中的⼀些概念:
code review 分⽀
如果我们将之前的 develop 称为** develop分⽀,new_feature_branch 称为 feature 分⽀,我们现在引⼊⼀个新的概念,叫做**code review 分⽀。
Minor change 时,会关系到** develop分⽀和 **code review 分⽀,执⾏的 land 命令为 arc land code_review_branch –onto develop
Feature Development 时,会关系到** develop分⽀、 feature 分⽀、 **code review 分⽀,执⾏的 land 命令为 arc land code_review_branch –onto new_feature_branch。之后会在 develop 上执⾏ git merge –no-ff new_feature_branch。
依赖性 revision 和 独⽴性 revision
在 crb1(code review branch 1) 上建⽴⼀个 revision 之后,
如果接下来的开发依赖这个 revision 的修改,需要在 crb1 基础上,再 git co -b crb2。然后在 crb2 上执⾏ arc diff –create 建⽴ revision,这个叫依赖性 revision。
land 依赖性 revsion 时,需要按依赖顺序,依次land,不能跳跃。可以⽤ revision id 来辨别顺序。如果跳跃 land ⼀个revision id更⼤的依赖性 revsion,会将它所依赖的 revision 也push上去。
如果接下来的开发不依赖这个 revision 的修改,从最新的 develop 或者 feature 分⽀上,co 新的code review branch即可。
配置⽂件及命令详解
上⾯这个流程⾥⾯的命令的⾏为,依赖⼀些 .arcconfig 中的⼀秋分节气的含义是什么
些配置。我们⽬前的配置⽂件为:
{
"phabricator.uri" : "10.100.16.251:8080",
"editor": "vim",
"ba": "git:HEAD^",
"default": "develop",
"arc.land.update.default": "reba",
"history.immutable": fal
}
还有其他的⼀些配置,可以⽤arc get-config –verbo查看。
这⾥的配置是什么意思,后⾯我会结合下⾯命令讲⼀下。
arc diff –create
它是⼀个⽐较重要的命令,这个是关于它的。
–create,不要省略,diff默认是create、还是update,是依情况⽽定,不好判断。
由于我们的配置⽂件,这个命令实际执⾏了 git commit -a;arc diff –create HEAD^。
省略git commit -a,是因为arc diff会帮你运⾏这个命令,之后才真正运⾏arc diff,diff的rang,也是在commit之后才确定。
省略HEAD^,是因为 .arcconfig 中的 “ba”: “git:HEAD^”“,”ba”的值是⼀些 rule,其他的 rule 还有”ba”:
“git:merge-ba(origin/develop), arc:prompt”等。
这个参数或者设置的意思是,HEAD^ 为arc diff 的 the beginning of the range。
>
In Git and Mercurial, many arc commands (notably, arc diff) operate on a range of commits beginning with some commit you specify and ending with the working copy state.
Since the end of the range is fixed (the working copy state), you only need to specify the beginning of the range.
arc land code_view_branch
arc land code_view_branch实际执⾏了 arc land –onto develop –squash –revision DXX –update-with-reba。
省略 –onto develop,是因为 .arcconfig 中的 “default”: “develop”。
省略 –update-with-reba,是因为 .arcconfig 中的 “arc.land.update.default”: “reba”。
省略 –squash 是因为
.arcconfig 中的”history.immutable”: fal。如果为 true,对应 –merge。
这些参数和配置的含义是什么呢?这就需要描述⼀下 arc land 这个命令执⾏的内容。⼤家可以看看某次 arc land 的输出:
Can@bogon:~/Workspace/zuche another_code_view_branch$ arc land code_view_branch --onto develop
Switched to branch develop.
The following commit(s) will be landed:
996e636 remove wechat sdk
Switched to branch deletefile. Identifying
Landing revision 'D46: remove wechat sdk'...
Rebasing code_view_branch onto develop
Current branch code_view_branch is up to da本末倒置的拼音
te.
Counting objects: 4, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4南瓜的功效与作用
), 585 bytes | 0 bytes/s, done.
Total 4 (delta 2), reud 0 (delta 0)
To git@10.3.4./zuche.git
a25fb28..481e168 develop -> develop
Cleaning up
(U `git checkout -b 'code_view_branch' '996e636bf77b84e3f9795eadc6e241006c6639a9'` if you want it back.)
Switched back to branch testontest.
Done.
我是在 another_code_view_branch 上执⾏的 arc land code_view_branch,也就是是说,arc land 的对象是 branch,如果不传⼊,默认为当前branch,因为我在another_code_view_branch上⾯,所以必须传⼊。
从输出可以看到这个命令的内容为:
1. ⾸先 switch 到 develop,然后 git pull 或者 git pull –reba。
如果不带–onto develop,会读取 .arcconfig 中的 default 配置项。
是 pull 还是 pull –reba,参见 arc get-config –verbo 中 arc.land.update.default,或者 arc help land 中 –update-with-merge、–update-with-reba 解释。
2. 然后 git reba 或 merge code_review_branch 到 develop 分⽀。
参见 arc get-config –verbo 中 history.immutable,或者 arc help land 中 –merge、–squash 解释。squash 是 reba 的⼀种,详细可以参考这个。
3. git push origin develop。
4. git branch -D code_review_branch。
arc patch revision-id
这个命令是 code reviewer ⽤来 review 代码的,⽹页端 review 代码只能静态看,⽽ patch 代码后,
可以在IDE⾥⾯ reivew 代码,并且可以运⾏起来动态调试。
运⾏这个命令前,要保证本地的develop或feature,包含revision所在分⽀的起始commit,最好先在develop或者feature执⾏git pull –reba。
要点和规范
⼀个rivision只对应⼀次commit,并且只对应⼀个code review branch。
操作⽅⾯:
⽤arc diff –create替代git commit
⽤arc land替代git push
当你发现⾃⼰diff的代码有⽐较⼤的问题,修改需要review:
如果还没有被accepted,应该通知reviewer,避免他已经开始 review 了。然后进⾏arc diff —update。
如果已经被糖醋里脊做法
accepted,应⾛新建依赖性revision。因为update ⼀个 Accepted 的 revision,它的状态不
会变成 Need Review。
为什么会这样,下⾯有说明。
当⼀个你review代码时发现⼀些⽐较⼩的问题,⼩到即使改了,你不想再review了,应该accept,并添加对这⽐较⼩的问题的comment。
这样做是避免不必要的打回,给作者带来⽓馁的情绪。
所以才有这样的设定:update ⼀个 Accepted 的 revision,它的状态不会变成 Need Review。⽽⼀个accepted的revision,作者还是可以进⾏minor change 的,并且是不需要review 的。
官⽅有。
Although this behavior is configurable, we think stickiness is a good behavior: stickiness encourage authors to update revisions when they make minor changes after a revision is accepted. For example, a reviewer may accept a change with a comment like this:
Looks great, but can you add some documentation for the foo() function before you land it? I also caught a couple typos, e inlines.
If updating the revision reverted the status to “Needs Review”, the author is discouraged from updating the revision when they make minor changes becau they’ll have to wait for their reviewer to have a chance to look at it again.
⼀般情况下,所有改动都是需要review的,即使⽐较⼩,也怕不⼩⼼修改了其他内容。
经验技巧
arc cover 命令
arc cover,能列出当前working tree到diff的beginning of the range,之间所涉及到的⽂件的之前的其他改动者。有点拗⼝,简单来说,就是推荐给你⼀些reviewer。
我们配置diff的beginning of the range为HEAD^,修改代码后,直接运⾏arc diff,⾸先会协助你commit,然后才真正diff,此时diff就仅仅包含你的这次commit的代码。
Commandeer的⽤法
Commandeer乃code review神器,但需要对git和arc⽐较熟多情况下才能⽤。下⾯我描述下它的威⼒:
1. 刚才fanshen提了D121给我review,然后我arc patch D121,然后git ret HEAD^,⽤xcode看看修改。
2. 发现⼀些有些问题,⼼想着这些问题其实我直接改了更快些,然后update D121,再让fanshen看看。
3. 修改好后我在arc diff —update D121,提⽰:You don’t own revision D121 ‘去掉⼏个不必要的IfDEBUG,修改DDLog’.
You can only update revisions you own. You can ‘Commandeer’ this revision对虾的做法
from the web interface if you want to become the owner.
4. 按照第3步提⽰,我Commandeer了D121,发现我和fanshen的⾓⾊互换了,在执⾏arc diff –update D121成功。
5. 然后我发现有些地⽅我改错了,继续修改并arc diff –update D121。
6. fanshen通过后,我来land。
查看revision每次的update
我们有个规范是⼀个rivision只对应⼀次commit。为什么呢,这⾥解释⼀下,rivision可以多次update,phabricator的code review界⾯能显⽰每个update的修改,但如果rivision有多次commit,⾸先,代码粒度有些⼤,其次,phabricator并不提供查看每次commit修改的功能。
关掉已经review过到⽂件的diff界⾯
已经review过的,可以通过view options,选Collap File,关掉这个⽂件的diff界⾯。
Inline comment
按住⿏标左键拖动选择⼀或多⾏:
提交develop或者feature分⽀上的修改
有时候因为某些原因,忘记创建code review branch直接在develop上尝试修改代码,修改好后想diff,可直接git co -b crb创建,因为没git add。