推荐阅读：开发工程师、测试、技术管理者
本文概述：代码质量涵盖的范围之广可谓触及产研流程的方方面面，多年来代码质量相关的方法论和依此产生的工具更是不胜枚举，本文我们尝试从 CodeReview视角介绍如何在企业内如何保证代码质量，落地小步快跑评审。
你将获得：
∙ “好”的提交具备的3个条件
∙ 提交编排4个基本原则
∙ 基于补丁版本的评审实践推荐
在代码评审的诸多方法和实践中，“小步快跑”一直是一个经常被提及的词语，原因主要有以下几点：
∙ 及时发现问题：及时发现问题，避免问题的积累
∙ 降低修正成本：如果一次代码评审设计的代码量较多或者内容不聚焦，那么发现问题后进行修正或重构成本较高。
∙ 研发协作效率：让评审的参与者们，可以每次进行一小部分工作，降低评审压力和难度，最终完成评审过程。
1. “小步快跑”为什么难以落地?
1） 什么是“小步”
我们将“小步快跑”拆分为两个部分来看，首先我们来看“小步”。从编写代码的角度来看，“小步”就是指每一次代码提交的内容要原子化，并且内容尽可能的少，即：一个提交做且只做一个事情。提交能否做到原子化，会直接影响后续代码评审、代码合并以及持续集成的效率，是最容易被忽略，却又最重要的前提之一。
成为更好的开发者，从学习做好一个提交开始
那么，一个“好”的提交具体应该如何衡量呢？我们认为主要应该体现在三个方面：
∙ 第一：提交做小
∙ 第二：提交做对
∙ 第三：提交做好
a）提交做小
提交做小就是将要解决的问题解耦：“Do one thing and do it well”。一些质量较为严格的项目的提交通常都很小，每个提交只修改一个到几个文件，每次只修改几行到几十行。找一个你熟悉的开源项目，在仓库中执行下面的命令，可以很容易地统计出来每个提交的修改量。
$ git log --no-merges --pretty= --shortstat 2 files changed, 25 insertions(+), 4 deletions(-) 1 file changed, 4 insertions(+), 12 deletions(-) 2 files changed, 30 insertions(+), 1 deletion(-) 3 files changed, 15 insertions(+), 5 deletions(-)
但是，在很多项目中则不是这样，有时候一个提交动辄修改成百上千的文件，涉及成千上万行的源代码，这样的提交很难进行测试，代码评审也几乎难以进行。
在开发过程中，程序员一旦进入状态，往往才思如泉涌，不经意间就写出一个大提交。比如：
一次向 Git 贡献代码时，提交还不算太大，就被 Git 的维护者 Junio 吐槽，要求拆分提交，便于评审：
I think this patch should be in at least two parts:
Introduce the two-phase "collect in del_list, remove in a
separate loop at the end" restructuring.
(optional, if you are feeling ambitious) Change the path that is
stored in del_list relative to the prefix, so that all functions
that operate on the string in the del_list do not have to do
*_relative() thing. Some functions may instead have to prepend
prefix but if they are minority compared to the users of
*_relative(), it may be an overall win from the readability's
point of view.
Add the "interactively allow you to reduce the del_list" bit
between the two phases.
关于提交拆分操作方法，可以扫码观看 Codeup 团队的相关视频介绍：
b）提交做对
“好的文章不是写出来的，而是改出来的”， 代码提交也是如此。
程序员写完代码，往往迫不及待地敲下：git commit，然后发现提交中少了一个文件，或者提交了多余的文件，或者发现提交中包含错误无法编译，或者提交说明中出现了错别字。
Git 提供了一个命令：git commit --amend 来把此次更改附加到当前提交上, 防止我们做的是同一件事情，但是产生了多余的提交，从而破坏了提交的原子化。
如果你发现错误出现在上一个提交或其他历史提交中怎么办呢？比如发现历史提交a1234567 中包含错误，我们可直接在当前工作区中针对这个错误进行修改，然后执行下面命令git commit --fixup a1234567。你会发现使用了--fixup 参数的提交命令，不再询问你提交说明如何书写，而是直接把错误提交a1234567 的提交说明的第一行拿来，在前面增加一个前缀“fixup!”，如下：
fixup! 原提交说明
那么当开发工作完成后，待推送/评审的提交中出现大量的包含“fixup!”前缀的提交该如何处理呢？
如果你执行过一次下面的命令，即针对错误提交 a1234567及其后面所有提交执行交互式变基（注意其中的 --autosquash 参数），你就会惊叹 Git 设计的是这么巧妙：
$ git rebase -i --autosquash a1234567^
交互式 rebase弹出的编辑器内自动对提交进行排序，将提交 a1234567 连同它的所有修正提交压缩为一个提交， 这样我们就完成了针对历史提交的修正，让提交保持原子化。
c）提交做好
仅仅做到提交做小、提交做对，往往还不够，还要通过撰写详细的提交说明让评审者信服，这样才能够让提交尽快通过评审合入项目仓库中。几年前，我们无意发现 Git 服务器上的一个异常，最终将问题定位到 Git 工具本身。整个代码改动只有区区一行, 但你能猜到提交说明写了多少么？写了20多行！
提交：receive-pack: crash when checking with non-exist HEAD
receive-pack: crash when checking with non-exist HEAD If HEAD of a repository points to a conflict reference, such as: * There exist a reference named 'refs/heads/jx/feature1', but HEAD points to 'refs/heads/jx', or * There exist a reference named 'refs/heads/feature', but HEAD points to 'refs/heads/feature/bad'. When we push to delete a reference for this repo, such as: git push /path/to/bad-head-repo.git :some/good/reference The git-receive-pack process will crash. This is because if HEAD points to a conflict reference, the function `resolve_refdup("HEAD", ...)` does not return a valid reference name, but a null buffer. Later matching the delete reference against the null buffer will cause git-receive-pack crash. Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
欣赏完这个提交，就让我们来看看关于提交说明的一些约定俗成的规定：
∙ 提交说明第一行是提交标题，是整个提交的概要性描述。提交标题的长度要求：尽量不要超过 50 个字符。 这是因为对于像 Linux、Git 这样的开源项目，是以邮件列表作为代码评审的平台，提交标题要作为邮件的标题，而邮件标题本身有长度上的限制。虽然我们很多日常开发是企业内的项目，但是我们应该遵守这一点，有益而无坏。
∙ 提交标题使用英文，尽量不要出现中文。这是因为一些 git 工具如 git format-patch 将提交转换补丁，或以邮件方式交换提交补丁的时候，会丢失中文（中文转换为空白字符！）
∙ 建议在提交标题中添加前缀，对改动范围进行区分（例如用模块名做前缀：receive-pack:）。
∙ 不要在提交标题后面添加标点符号（如句号.）。一个原因是提交标题的长度要求，不要浪费。
∙ 提交标题后的空行。必须要在提交说明的第一行（subject）和后续的提交说明（body）中间留一个空行！如果没有这个空行，很多 Git 客户端会将连续几行的提交说明合在一起作为提交的简要描述（git log --oneline）,这样显然查看起结果来太糟了。
∙ 提交说明也有长度的限制，最好以72字节为限，超过则断行。提交说明主体中要写什么内容呢？ 例如：本次提交要解决什么问题？如何解决的？为什么这么做是最合理的。
∙ 签名区: git commit 命令支持 -s 参数，会自动在提交说明最后添加 "sob" 签名。为什么要在提交后面添加签名呢？因为一个提交的元信息中只有作者（author）、提交者（committer）两个字段，而一段代码的诞生，参与的人往往不止于此，还可能有问题报告者（Reported-by）、代码评审者（Reviewed-by）、上游 Committer 的签名（Signed-off-by）。为此一些开源项目（如 Git、Linux）的一个约定俗成的习惯，是在提交的最后加上签名，每个贡献者一行，从上到下可以看到这段代码诞生的过程。对帮助过你的人致谢吧，加上对应的代码签名。
2）“快跑”：提交之间的逻辑化组织
对于代码评审来说，光把单个提交做好还不够。我们认为：代码评审要关注过程，要由远及近地看每一个提交，不能只看前后两个版本之间的文件差异。有人认为这样的评审方式多此一举，并认为这样做可能是在浪费时间。有的时候，给一个提交不规范的开发者做代码评审，的确头疼又浪费时间：看到一个提交中的代码问题，花了几分钟写评论，然后发现下一个提交中这个问题被修正（fixup）了，这样的情况时有发生，让人无奈。
a）提交之间组织或编排，往往被人忽略
一些开发者会将 提交(git commit)和推送(git push)混淆，commit 实际上是我们本地编写和组织改动的过程，而推送实际上是本地与远端 Git 服务器之间的交互行为。我们都知道，从开发到不断修正的过程中，可能经历多次git push， 每一次推送都可能包含多个 commits 形成了一次要发送的版本，推送的版本一般被称之为补丁（Patch）。
在一个补丁中包含多个提交的情况时有发生，那么接下来的问题是，这些提交应该如何在补丁中进行组织和编排呢？
b）提交编排的基本原则
∙ Cleanup提交
∙ Preparation提交
◦ Refactor提交
◦ Test cases supplyment 提交
∙ Implementation提交
∙ Test cases 提交
∙ Documentation提交
Cleanup 提交
首先，Cleanup可以理解为当我们真正开始开发一个补丁（patch）之前，可能我们顺带发现了之前相关联代码的一些问题，为了更好的实现我们的特性或者缺陷修复，我们需要尽量修复之前的问题，例如缺陷或格式化的问题，所以，Cleanup 类的提交都应该在率先完成。Cleanup 可能包括多个提交，比如一个提交用于修复代码格式化问题，另一个提交用于修复错误的文档和注释等等。
Preparation 提交
在完成Cleanup 提交之后，我们可能还需要进行一些重构工作，以及如果是缺陷修复的情况可能还务必需要补充对应的测试（对于重构来说最好也需要检查测试是否需要补充）。 Preparation 与 Cleanup 提交，通常我们称之为前置提交。
前置，是因为它们确实很有必要提前就拆分出来（因为实际上后面发现需要拆分时，拆分工作通常十分困难）。其次，这些提交的引入不应该 break 原有的功能和行为，这样无疑可以让评审者的评审效率以及作者后续的补丁编写效率大幅度的提升。Preparation 也可能包括多个提交，比如一个提交用于修改不恰当的函数名称，另一个提交用于抽象和提取公共函数等等。
让我们通过前置提交的方式，更好的体现我们的解耦功力，让“快跑”成为可能吧！
Implementation 提交
接下来，就开始真正的实现层面的提交了，不论是缺陷修复或者是特性开发，Implementation 提交都有可能包含多个，我们需要依次实现即可，比如我们需要对某业务提供一个新的 API，按照实现顺序可能包含如下的几个提交：
∙ IMP提交 1：进行 API 接口的设计（此时可以推送补丁 A 到远端，评审者收到通知后便会进行评审）
∙ IMP提交 2：进行 DAO 层的逻辑实现（此时可以推送补丁 B 到远端，评审者收到通知后便会进行评审）
∙ IMP提交 3：进行 Service 层的逻辑实现（此时可以推送补丁 C 到远端，评审者收到通知后便会进行评审）
∙ IMP提交 4：进行 Controller 层的逻辑实现（此时可以推送补丁 D 到远端，评审者收到通知后便会进行评审）
我们可以发现， 当我们把提交的内容尽量解耦，同时让每个提交具备原子性之后，频繁发送最新的补丁（基于补丁（patch）的评审工作流）就变成了极为优秀的实践方式。一方面，可以及时的告知评审者们我工作的进展，另一方面评审者每次评审的内容也可以基于递进式的方式，变得更加可控和高效。
最后，对于评审作者，我们也会更加及时的获取各方评审者的反馈，让我们得到更多提前修正的机会，形成较低成本的正向评审循环。
进一步的，就上面的例子来说，试想如果在 API 接口的定义上存在问题，那么在补丁版本 A 对应的评审上就可以提前得知。试想，如果我们在周五下班前一口气发送了包含如上 4 个提交的补丁 A，结果却在代码评审时被告知 API 接口的设计存在问题，那么最坏的情况下需要修改 IMP 1~4 的 4 个提交，这显然糟糕透了，因为我明明在周一的时候就完成了 API 接口的设计！
通过上面的例子，有没有抓到了“快跑”的一点诀窍呢？通过让工作步骤“解耦”并“频繁”寻求反馈，“小步快跑”远没有我们想象中那么难。
Test cases 提交 和 Documentation 提交
针对我们的补丁，我们通常需要在本工程中进行测试用例的补充，我们可以单独的拆分对应的提交，但是这并不是必须的。有些时候，测试用例的改动以及文档的改动可能会连同 Implementation 提交压缩在一起，这在一些情况下是非常必要的，尤其在添加或者删除某个特性的时候，例如：
提交：
https://github.com/git/git/commit/425b4d7f47bd2be561ced14eac36056390862e8c
这个提交中既包含一个新的选项的支持，同时在提交中还包括测试用例和文档的改动，他们之前有强关联性，因为他们都是 “Do One Thing”。对于 Git 开源项目来说，在测试用例和文档质量上的要求尤为严格（上述提交只改了 7 行代码，但是对应的测试用例和文档改动却有 135行！），从而每一个提交都可以通过完整的测试用例。
对于可能存在功能性break 的情况时，连同测试用例和对应的改动压缩为一个提交是非常必要的，评审者可以知晓改动在向后兼容性和破坏性方面是否存在问题。
而对于文档改动的提交也同理，很多工程的文档同样需要构建，例如提供对外的 API 文档等，这部分内容同样要尽量保持同步修改。
但有些时候，压缩在一起也不是必须的，比如可能补丁本身的工作就编写测试用例和文档。
c）基于补丁（Patch）版本的评审工作流
刚才我们通过介绍在一个补丁中进行提交编排的原则，引出了基于补丁版本的评审工作流的概念。在“小步快跑”的方法实践中，了解这种工作流的运转方式其实非常重要，主要包括：
∙ 根据代码评审意见，代码评审的作者每次推送会产生新的patch。
∙ 代码评审者以 patch 为粒度，了解编排好的提交，对 patch 的演进进行最为直观了解。
∙ 支持老 patch 与当前 patch 之间的文件改动。
∙ 对于已经 review 过的历史 patch，同样支持通过指定 patch 区间评审特定版本之间的变更。
∙ 保存历史 patch 衍生的全部数据（测试结果、评审意见、评论等），可追溯特定 patch 的评审上下文信息。
云效 Codeup 新版代码评审支持基于补丁（patch）的评审工作流
d）提交编排在补丁之间的变化
我们很难做到每次补丁中的提交编排都非常正确，我们仍旧按照上面的实现 API 的例子来说明：
∙ IMP提交 1：进行 DAO 层的逻辑实现（此时可以推送补丁 A 到远端）
∙ IMP提交 2：进行 Service 层的逻辑实现（此时可以推送补丁 B 到远端）
∙ IMP提交 3：进行 Controller 层的逻辑实现（此时可以推送补丁 C 到远端）
假设我们作为评审者，从补丁 C 开始评审，我们根据上述的补丁情况，我们希望开发者可以将提交 3 拆分为两个提交，将 API 的接口设计和具体实现进行解耦, 评审作者根据我们的评审意见，推送了补丁 D 即：
∙ IMP提交 1：进行 API 接口的设计（此时可以推送补丁 A 到远端）
∙ IMP提交 2：进行 DAO 层的逻辑实现（此时可以推送补丁 B 到远端）
∙ IMP提交 3：进行 Service 层的逻辑实现（此时可以推送补丁 C 到远端）
∙ IMP提交 4：进行 Controller 层的逻辑实现（此时可以推送补丁 D 到远端）
其中在最新的补丁 D中，评审者对于提交 2 和 提交 3 无需关注， 因为他们对比上次评审的版本没有变化， 只需要关注拆分出来新的提交1 以及 提交4， Git 支持range-diff 命令来查看补丁之间的提交差异：
$ git range-diff 34d57b4..3cfc130 34d57b4..ce628af
-: ------- > 1: 5b6048e api: design and mock membership GET API
1: 63a70e5 = 2: 8318c31 dao: introduce query membership INF
2: 7ab068a = 3: c11a747 service: introduce getQueryMembershio INF
3: 3cfc130 < -: ------- api: introduce membership GET API
-: ------- > 4: ce628af controller: query membership API to mature
可以看到对比旧的补丁，在新的补丁中新增了提交1 和提交4 ，在旧的补丁中删除了提交3。通过首先查看和对比补丁之间的提交差异，对于整个评审效率的提升无疑是巨大的。一方面，评审者关注的改动范围得到的缩小，而不用每次都通过浏览全量的文件改动来识别差异。另一方面，可以更加了解评审作者的编码思路和设计，帮助我们给出更加正确和准确的评审建议。
云效 Codeup 新版代码评审支持查看补丁之间的提交维度的改动
当我们将提交做好，并且结合基于补丁的代码评审方式来实现工作流的时候，不论是对于作者还是评审者的效率提升都是非常显著的。但是要注意，这二者缺一不可，是让这一切发生基础。实际上，二者的收益不止于此，例如在持续集成方面也将带来巨大的正向收益。
