代码评审(Code Review)是极限编程中用来保障代码质量的有效方式。而拉式请求(Pull Request)的模式,在 GitHub 网站作为分布式代码协作的一种模式被成功运用之后,也很快成被很多团队引用到 Git Flow 中的流程中。Git Flow 中由于特性分支的存在,因此在特性分支(feature 分支)往开发分支(develop)合并时,就为使用 Pull Request 提供了时机:当 Pull Request 被通过时,代码才被合并进开发分支。

主干开发(Trunk Based Development)的模式中,想采用 Pull Request 模式来辅助代码评审的动机是想要有一个简单易用的工具来组织代码评审的内容,记录评审会议期间团队对代码修改的建议,并追踪后续修复过程。但在主干开发的团队中,由于没有功能分支的存在,所以“技术上”并不满足创建 Pull Request 的前提条件。因此,采用主干开发的团队一直默默使用一些额外的工具和方法来解决上述问题。由于只是技术上的问题,那么解决起来也就不麻烦了。通过创建临时的分支,在临时分支上创建 Pull Request 即可在主干开发的团队中使用 Pull Request 来做代码评审。

Pull Request

下图是 GitHub 网站上关于 Pull Request 的示意图:

image

在非主干开发的团队中,图中上面的直线即为团队主干(即 GitHub Flow 中的 master 分支,或者 Git Flow 的 develop 分支),代码合并入团队主干之前,开发人员在自己的分支中开发,做了若干次的提交(commit),然后在功能开发完成之后,准备将这些提交合并到团队主干中去。接下来就打开代码协作网站(比如 GitHub),创建一个 Pull Request(是的,也可以为同一个代码库中的不同分支之间创建 Pull Request),并等待评审通过后,代码就可以被合并到团队主干中去。

在 Pull Request 的页面,评审者、代码作者及其他人员可以就代码的细节展开讨论,提出建议修改的地方,代码作者通过继续向自己的分支提交代码来达到评审者的要求,最终代码被合并到团队主干。下图(来自 GitHub 文档)是 GitHub 上对 Pull Request 展开讨论的示意图,在该界面可以看到拉式请求的简介,以及提交列表和对文件的修改细节:

image

作为一种代码提交过程的协作流程,Pull Request 模式与广为使用的 Git Flow 结合的很好,因此在很多代码协作工具中都提供了这样的功能,除了 GitHub,在 TFS、gitlab 或者 gogs 这些同类软件中都提供了类似的功能。

主干开发中的代码评审

不过,ThoughtWorks 更推荐主干开发,并且从持续集成的有效性等方面考虑认为 Git Flow 是有害的。简单来说,主干开发就是所有开发人员直接将代码提交到主干分支上,而不以团队成员或功能等其他方式创建临时或长期分支。

image

很多人可能担心,那大家在活跃开发之中的时候,代码都往主干上提交,不是相互影响、一片混乱吗?理论上是很有可能的。而主干开发得到推荐最直接的原因就是,这是最有利于持续集成的一种代码模式。同时,持续集成也是能确保代码不会陷入混乱的有力措施。不过,光有持续集成是不够的。极端情况下,一个使坏的开发人员可以通过故意不写测试,或者删掉已有单元测试来避免触发持续集成的检查,从而给代码库中增加错误的代码。为了保障代码的质量,团队共同开展的代码评审也是必要的措施。

有了主干开发的加持,团队希望只要持续集成处于成功状态,提交代码应该是越早越好。我们不希望因为评审过程而失去这种自由,所以评审不应该是阻碍代码进入主干的一种“流程”,而只是对已提交代码的一种确认。

所以,我们几乎从来没有考虑过 Gerrit 这样的流程辅助软件。团队曾一直使用很原始的方式来进行代码评审,所有人围着同一台电脑(或大屏幕),在电脑上使用 Gitk 等代码历史查看工具挨个查看提交中所包含的变更,并就修改细节进行讨论。这样做的好处是,评审过程非常轻量级,只要用一个变更历史查看工具就可以做评审。但一个个提交去看,也带来一些效率问题。所以也陆续尝试过一些能把多次提交中的变更的差异合并显示的工具(比如 WebStorm 等 JetBrains 系 IDE 的变更历史查看工具)来提高一些评审的效率。

image

一开始,大家很享受这种轻量级的代码评审。一个小时下来,整个团队前一天的代码都看完了。期间由一个人拿便签纸记下收到的反馈,再由开发人员各自领取属于自己的条目回去修复。不过,一段时间以后人们偶然发现,一些此前在代码评审中讨论过的问题,最终还是引发了——比如在 QA 环境发现了相关的缺陷。也就是说,这些在代码评审过程中提出的修订意见并没有得到及时落实。一个便签条,贴到屏幕上,如果当时快速修复了这些问题,就很高效。但如果当时被其他工作打断而没有及时处理,后面可能就忘记了。我们缺少一个在评审完成后的跟踪和确认机制。

image

在主干开发中使用 Pull Request

有同学再次提起了 Pull Request,我们此时发现它不光是一种代码协作流程,它实际上也提供了在协作过程中承载信息、跟踪结果的能力。Pull Request 页面中的讨论、注释,以及标记等功能,可以很好地用来记录和跟踪代码评审的内容。待下次评审,再来检查上次评审过的条目,以确认之前讨论过的修订意见都被妥善处理了。

在确认了要使用 Pull Request 模式之后,挡在我们面前的还有两个问题:

  • 主干开发模式中只有一个分支,并没有功能分支,因此没有可用于创建 Pull Request 的条件
  • 即使有分支,如果要等 Pull Request 评审通过才能合并到主干,那么也是不小的延迟,与持续集成的思路不符

对于没有分支可用于创建 Pull Request,这并不麻烦,只需要创建临时分支即可。第二个问题,我们准备创建临时分支、推送到远端之后,创建了 Pull Request 之后就立即将该分支合并到主干。这时 Pull Request 会自动被关闭,不过这并不影响它记录变更、支持评审活动的功能。既然代码都已经合并到了主干,临时的分支也没有了用途,所以也可以删除了。

image

如果嫌每次提交代码时都有这么多步骤太繁琐了,可以写一个脚本把整个过程自动化起来。GitHub、TFS 等代码协作平台都提供了命令行工具以及 API,这样的脚本写起来并不麻烦。

有了 Pull Request,结合代码协作平台(比如 GitHub)提供的标记(label)功能,可以进一步优化出一套工作流程出来:

  1. 需要提交代码的同学创建一个 Pull Request,并将其标记为 pending-review(即“待评审”)
  2. 在第二天的评审活动上,找到所有已经标记为 pending-review 的 Pull Request
  3. 在这些 Pull Request 查看修改记录,并讨论形成修订意见
  4. 评审完成后,去掉 Pull Request 上的 pending-review 的标记,同时如果有修订意见形成,则标记为 pending-fix(即“待修复”)
  5. 在第三天的评审活动上,首先快速检查第二天形成的修订意见是否已经修订完毕,再开始当天的评审。确认已修订完毕后,去除 Pull Request 上的 pending-fix 标记

image

通过这样一番“折腾”,就可以在主干开发模式下利用 Pull Request 来管理代码评审的过程了。整个过程并不需要引入其他的工具,配置起来非常简单;最终的操作流程对原来的主干开发的开发过程的影响很小,接纳它的成本很小。经过一段时间的实践,我们发现这样的轻量级的流程很快受到了团队的欢迎,并得到了较好的坚持。