带你读《代码管理实践10讲》——四、打破代码评审“小步快跑难落地”的魔咒

简介: 带你读《代码管理实践10讲》——四、打破代码评审“小步快跑难落地”的魔咒

推荐阅读:开发工程师、测试、技术管理者

本文概述:代码质量涵盖的范围之广可谓触及产研流程的方方面面,多年来代码质量相关的方法论和依此产生的工具更是不胜枚举,本文我们尝试从 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 团队的相关视频介绍:

 

image.png

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 个字符。 这是因为对于像 LinuxGit 这样的开源项目,是以邮件列表作为代码评审的平台,提交标题要作为邮件的标题,而邮件标题本身有长度上的限制。虽然我们很多日常开发是企业内的项目,但是我们应该遵守这一点,有益而无坏。

∙        提交标题使用英文,尽量不要出现中文。这是因为一些 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)。为此一些开源项目(如 GitLinux)的一个约定俗成的习惯,是在提交的最后加上签名,每个贡献者一行,从上到下可以看到这段代码诞生的过程。对帮助过你的人致谢吧,加上对应的代码签名。

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 的评审上下文信息。

image.png

云效 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。通过首先查看和对比补丁之间的提交差异,对于整个评审效率的提升无疑是巨大的。一方面,评审者关注的改动范围得到的缩小,而不用每次都通过浏览全量的文件改动来识别差异。另一方面,可以更加了解评审作者的编码思路和设计,帮助我们给出更加正确和准确的评审建议。

 

image.png

云效 Codeup 新版代码评审支持查看补丁之间的提交维度的改动

 

当我们将提交做好,并且结合基于补丁的代码评审方式来实现工作流的时候,不论是对于作者还是评审者的效率提升都是非常显著的。但是要注意,这二者缺一不可,是让这一切发生基础。实际上,二者的收益不止于此,例如在持续集成方面也将带来巨大的正向收益。

目录
相关文章
|
3天前
|
存储 运维 jenkins
放弃"Jenkins"的种种理由,期待更好赋能研发的"持续交付平台"
Jenkins 很酷,但是不完美,有历史局限性造成的问题。本文仅从“如何更好给研发团队赋能的角度”,剖析Jenkins, 探讨理想的持续交付平台, 不带货无广告~
|
3天前
|
运维 安全 Cloud Native
解读平台工程,DevOps真的死了吗?不,它只是换了个马甲而已,依然是DevOps的延续
最近平台工程这个概念越来越火爆,Gartner 的预测,到 2026 年,80% 的软件工程组织将拥有平台工程团队,来提供内部服务、组件和应用程序交付工具,作为可重复使用的资源。本篇文章将带你走进平台工程,了解它的起源和解决的问题。
104 0
|
3天前
|
设计模式 供应链 安全
如何在短频快的节奏中做好技术?业务开发必会的架构思维
本文提供一种业务架构设计模式:从业务&技术两个角度提炼出一个基础思维框架,供业务线开发同学参考。
如何在短频快的节奏中做好技术?业务开发必会的架构思维
|
6月前
|
安全 区块链 数据安全/隐私保护
链游开发流程需要经历哪些阶段?
链游开发流程需要经历哪些阶段?
|
3天前
|
运维 安全 开发工具
打破代码评审难落地魔咒,轻松构建基于代码评审的研发流程和文化
本文介绍小布快跑,高效协同,如何轻松构建基于代码评审的研发工作流。
|
3天前
|
运维 Devops 开发工具
PPT & 回放|打破代码评审难落地魔咒,轻松构建基于代码评审的研发流程和文化
代码评审的好处不言而喻,为何实际落地却困难重重? Git 和 Gerrit社区贡献者、云效Codeup开发者 滕龙认为问题主要出在流程工具问题、时间资源限制、自动化程度不足这3方面。 在昨天的直播中,滕龙给出了详细的解法,包含好的代码评审应该怎么做和如何选对工具高效落地2方面。
840 1
|
运维 Kubernetes 监控
基于 K8s 的交付难题退退退!| 独家交付秘籍(第三回)
经过仔细研究,我们发现秘籍中提到许多帮助解决交付问题的招式,而其中一个让我们印象很深,是关于在原有社区版容器底座 Kubernetes(以下简称 K8s)的基础上,对容器底座进行改进,可更好的服务于应用交付的招式。下面,请随我一起来看看您是否是那天选之人吧!
基于 K8s 的交付难题退退退!| 独家交付秘籍(第三回)
|
敏捷开发 前端开发 BI
好的每日站会,应该这么开 | 敏捷开发落地指南
高效落地敏捷开发,先从这3个关键活动着手。在敏捷迭代中,虽然迭代周期比较短,但依然需要对迭代过程进行有效跟进。如果在输入、过程、输出环节,没有要求,每日站会(迭代跟进)将会非常低效。好的每日站会,应该这么开!
1101 0
好的每日站会,应该这么开 | 敏捷开发落地指南
|
敏捷开发 数据可视化
手把手,带你用数据做好迭代复盘改进 | 敏捷开发落地指南
高效落地敏捷开发,先从这3个关键活动着手。带你用数据做好迭代复盘改进 ,数据说话,借助云效项目协作·Projex 高效开展迭代复盘高效落地敏捷开发。
799 0
手把手,带你用数据做好迭代复盘改进 | 敏捷开发落地指南