怎么做 Code Review

Code review 是人人都明白要做的东西,不过做得得心应手的不多。好的实践要解决两个问题:第一是发现 code 的问题;第二是把问题正确传达给所有参与者 (reviewers) 。

通常说 code review 工具就会提到 GitHub 的 pull request,或者 Code Collaborator。这些工具解决的是第二个问题。比如说怎么知道其他 reviewers 是否已经提出相同的问题。或者 author 对某个 reviewer 提出的问题是否有了回应,refine 的对不对。诸如此类问题,不能说不重要。但只是 code review 的两方面之一。交流问题的前提是发现问题。眼光局限于上述这些工具,就是以为大家在一起聊着聊着问题就被发现了。问题绝不是靠盯着 pull request 或者 Code Collaborator 的 change list 页面看看就能自然而然地被发现。哪怕仅仅是两三行改动也需要放到整个 code base 中去检验。最好的 review 环境是既有清晰的 code change visualization,又能在整个 code base 里进行检索,还可以自由地运行修改前后的 code。PR 和 CL 提供了不错的 visualization,但缺少对后两点的支持。

所以 code review 的第一步是要把修改后的整个 code base,而不仅仅是修改本身的 visualization,共享给 reviewers。这种共享不但要让 reviewers 拿到 code change,还要能「玩起来」—— 能编译,能运行,能加入自己的修改来验证建议。

Git 这样的提供 cheap branch 的版本系统很容易做到这种共享。而 Perforce 的 branch 很 expensive,通常是几个人的 sub-team 共享同一个 branch。所以早先用 Perforce 的团队做 code review 往往就走马观花了。其实大概五年前推出的 shelve 功能就是专门为了 code review 设计的。Perforce 的 pending change list 相当于只有一个临时 commit 的没有历史纪录的 short live branch,同样能提供类似 rebase 的功能。Shelve 则提供了共享这个临时 branch 的能力。

共享问题解决之后,回头看 visualization 的问题。Perforce 在 branching 方面的弱点反而让 visualization 略显容易,因为缺乏历史纪录,所以大多数 IDE 都能自动把 shelved change list 唯一的选择 —— uncommited vs. committed 进行不错的可视化。而面对 Git 就比较头疼,因为 code author 在自己的 branch 里可以想怎么搞就怎么搞,开三五个 sub-branch 然后 merge,或者从别人的 branch merge 乃至 cherry pick 都算是 common practice。其实解决的方法也很简单,在本地做一个 uncommitted merge,然后 review 这个 merge。

最后多说一句闲话,Git 实践多了会发现 uncommitted merge 的用处不限于 code review。比如说,当 repo 里的 branch 很多的时候,在 SourceTree GUI 里看 logs 的时候会选择某一个 branch 而不是 all branches。如果这时还想对比另一个 branch 的情况就可以做一个 uncommitted merge。不过我还没想到把这个 trick 推广到两个以上 branches 的方法。

发表评论

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / 更改 )

Twitter picture

You are commenting using your Twitter account. Log Out / 更改 )

Facebook photo

You are commenting using your Facebook account. Log Out / 更改 )

Google+ photo

You are commenting using your Google+ account. Log Out / 更改 )

Connecting to %s


%d 博主赞过: