title: "Codeup CR" date: 2023-07-05T14:01:31+08:00

draft: false

前言

代码质量涵盖的范围之广可谓触及产研流程的方方面面，多年来代码质量相关的方法论和依 此产生的工具更是不胜枚举，仅凭一篇文章很难盖全。所以本文我们从代码评审的切面入手， 尝试从 CodeReview 的视角介绍我们在云效 Codeup 产品评审能力的建设过程中，产生的一 些思考和实践。

1. CodeReview 的“小步快跑”为什么难以落地?

1.1 什么是“小步”

我们将“小步快跑”拆分为两个部分来看，首先我们来看“小步”。我们认为，“小步”就是指每 一次代码提交的内容要原子化，并且内容尽可能的少，即：一个提交做且只做一个事情，把 提交尽量做“好”。

1.1.1 成为更好的开发者，从学习做好一个提交开始

“好”的提交具体应该如何界定呢？我们认为主要应该体现在三个方面：

• 提交做小
• 提交做对
• 提交做好

1.1.1.1 提交做小

写小提交就是将问题解耦：“Do one thing and do it well”。开源项目的提交通常都很小， 每个提交只修改一个到几个文件，每次只修改几行到几十行。

找一个你熟悉的开源项目，在仓库中执行下面的命令，可以很容易地统计出来每个提交的修改量。

shell $ 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(-) 而我看到很多公司内的项目则不是这样，一个提交动辄修改成百上千的文件，涉及成千上万行的源代码。试问这样的提交能 Show 出来给人看么？

可是在开发过程中，程序员一旦进入状态，往往才思如泉涌，不经意间就写出一个大提交。 比如一次向 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 dellist relative to the prefix, so that all functions that operate on the string in the dellist 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 团队的技术视频进行了解： 《拒做无名之辈！写好提交，做一个有品味的程序员》。

1.1.1.2 提交做对

“好的文章不是写出来的，而是改出来的。” 代码提交也是如此。

程序员写完代码，往往迫不及待地敲下：git commit，然后发现提交中少了一个文件，或 者提交了多余的文件，或者发现提交中包含错误无法编译，或者提交说明中出现了错别字。

Git 提供了一个命令：git commit --amend 来把此次更改附加到当前提交上, 防止我们做 的是同一件事情，但是产生了多余的提交，从而破坏了提交的原子化。

如果你发现错误出现在上一个提交或其他历史提交中怎么办呢？比如发现历史提交 a1234567 中包含错误，直接在当前工作区中针对这个错误进行修改，然后执行下面命令 git commit --fixup a1234567。你会发现使用了 --fixup 参数的提交命令，不再询问你 提交说明怎么写，而是直接把错误提交 a1234567 的提交说明的第一行拿来，在前面增加一 个前缀“fixup!”，如下：

fixup! 原提交说明

当开发工作完成后，待推送/评审的提交中出现大量的包含“fixup!”前缀的提交该如何处理呢？

如果你执行过一次下面的命令，即针对错误提交 a1234567 及其后面所有提交执行交互式变 基（注意其中的 --autosquash 参数），你就会惊叹 Git 设计的是这么巧妙：

shell $ git rebase -i --autosquash a1234567^

交互式 rebase 弹出的编辑器内自动对提交进行排序，将提交 a1234567 连同它的所有修正 提交压缩为一个提交。

1.1.1.3 提交做好

仅仅做到提交做小、提交做对，往往还不够，还要通过撰写详细的提交说明让评审者信服， 这样才能够让提交尽快通过评审合入项目仓库中。几年前，发现 Git 服务器上的一个异常， 最终将问题定位到 Git 工具本身。整个代码改动只有区区一行：

提交：receive-pack: crash when checking with non-exist HEAD

你能猜到提交说明写了多少么？写了20多行！

``` 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）的一个约定 俗成的习惯，是在提交的最后加上签名，每个贡献者一行，从上到下可以看到这段代码诞 生的过程。对帮助过你的人致谢吧，加上你的代码签名.

1.2 “快跑”：提交之间的逻辑化组织

对于代码评审来说，光把单个提交做好还不够。我们认为：代码评审要关注过程，要由远及 近地看每一个提交，不能只看前后两个版本之间的差异。有人认为这样的代码评审多此一举， 认为这样可能是浪费时间。有的时候，给一个提交不规范的开发者做代码评审，的确头疼又 浪费时间：看到一个提交中的代码问题，花了几分钟写评论，然后发现下一个提交中这个问 题被修正（fixup）了，这样的情况时有发生，让人无奈。

1.2.1 提交之间组织或编排，往往被人忽略

我们都知道，在从开发到不断修正，最终可能经历多次 git push， 每一次中间都可能包 含多个提交， 我们可以把每一次推送对应到代码评审中的特定版本或者补丁（Patch）。。

既然包含多个提交的情况是常态， 那么这些提交应该如何组织和编排呢？

1.2.2 提交编排的基本原则

• Cleanup【先做】【靠后提交】
• Preparation
  • Refactor
  • Test cases supplyment
• Implementation
• Test cases
• Documentation【最后做】【靠前提交】

首先，Cleanup 可以理解为当我们真正开始开发一个补丁（patch）之前，可能我们顺带发现了之前相 关联代码的一些问题，为了更好的实现我们的特性或者缺陷修复，我们需要尽量修复之前的问题，不论是 新发现的缺陷还是格式化的问题，Cleanup的提交应该处于补丁中靠后位置。

体现开发者的设计功力，让“快跑”成为可能

我们都知道，跑是一步接着一步，没有人可以不迈出第一步而直接奔跑， 每一步与上一步都是承接关系。

步： 每一次发送的版本或补丁（Patch），其中可能包含 1 个或多个提交 跑： 每一次版本或补丁（Patch）， 都可以快速的定位差待评审内容， 同时如果待评审内容足够的“小”

1.2.3 通过 git rebase 进行提交编排

1.2.4 通过 git range-diff 理清提交维度的改动差异

1.3 基于 Patch 的评审工作流

代码评审以 patch 为颗粒度，每次推送会产生新的 patch 对于已经 review 过的历史 patch，支持通过指定 patch 区间评审特定版本之间的变更， 大幅提升 review 效率 将保存 patch 衍生的全部数据，方便追溯特定 patch 的评审上下文信息

2. 代码评审与持续集成的紧密结合

2.1 为什么越多的人倾向于在代码评审中进行持续集成？

通过代码评审中进行持续集成，主要有如下好处：

• codereview 通常具备一定的社区属性，参与评审的各个角色可以充分交流。

• 持续集成中产生的问题，例如 foo.bar 文件的第 20 行导致编译失败，可以比较顺滑的 与评审的代码内容进行准确映射。

• 代码评审通常是在代码真正合入主干之前的一环，在事前发生持续集成更能提早暴露问题。

2.2 git bisect 的作用

2.3 持续集成的扩展——三方自动化检查

通常在不同企业持续集成的诉求度也不尽相同，针对这种情况，如何更够更好的支持三方自 动化检查就非常重要，其中包括：

• 支持提交状态检查、三方应用检查等CI工具集成。
• 支持查看特定 patch 的自动化检查结果。
• 三方应用检查支持自定义markdown模版。

3. 轻评审与重评审流程

3.1 轻量级评审

轻评审即轻量级评审，适合将迭代速度放在第一优先级的产研团队，对代码质量和上线后的风险方面有一定包容度。

在研发资源极度紧张，业务需求井喷的场景下，质量和速度是鱼和熊掌不可兼得的事，很多初创企业的管理者或者是技术Leader都选择接受一定的技术债务，在研发流程上，不要求一定满足严格的评审合入准则， 在这种情况下，请评审是比较不错的选择。

3.2 重量级评审

重量级评审与轻量级评审采取不同的策略，通常常见于以下常见的场景：

1. 业务类：对于代码质量要求较高
2. 服务类：应用、中台类应用、工具等
3. 开源类：开源贡献场景，通常对于代码质量的要求较高

重量级评审通常对于质量上的流程把控比较严格，要求评审合入之前需要通过一系列的

3.3 灵活的卡点支持

不管是轻量级评审还是重量级评审，不同的企业对于代码评审的卡点支持上，通常主要关注通用性、扩展性和灵活性三个维度。

3.3.1 通用性

通用卡点主要包括：

• 支持代码冲突检测结果作为合并卡点
• 待解决评论作为合并卡点
• 评审人评审是否通过作为合并卡点

3.3.2 扩展性

扩展性主要体现在，根据自身业务诉求，在基础卡点类型基础之上，支持扩展平台内置或者三方接入的自定义卡点：

• 代码风格：支持code-style检测结果作为合并卡点。
• 代码测试：支持单元测试、集成测试、可交付测试等测试结果作为合并卡点。
• 代码检测：支持依赖包漏洞检测、代码漏洞检测、代码扫描检测、开源合规检测等检测结果作为合并卡点。
• 其他卡点类型。

3.3.3 灵活性

通常在一些特殊的场景下，尽管设置了代码评审的相关卡点，你可能仍旧希望在特定规则下，可以灵活的控制卡点的生效逻辑，包括：

• 可灵活选择卡点结果是否作为卡点，例如只希望代码风格的卡点进行检查和结果查看，但是不作为真正卡点阻碍代码的合并。
• 在紧急发布情况下，本地已经验证通过，希望临时将某些卡点检查自动跳过，从而尽快进行合并从而修复线上缺陷。

4. 评审协同效率

4.1 评审的发起与更新

基于主干模式下的推送评审流程

todo 贴视频

4.2 化零为整，评审操作更专注

将评审过程中各类操作进行聚合，提供统一视图进行评审操作 可以在统一评审视图中，再次 review和补充本次评审操作的关键内容

4.3 化繁为简，评审待办更直观

快速浏览和定位待解决评论 快速浏览和定位自动化检测问题 快捷键支持（即将上线）

4.4 化腐为奇，评审评论可追溯

• 支持行内评论、整体评论以及草稿评论等常见类型
• 支持追溯 outdate 评论
• 评论支持markdown格式，以及动态表情评论

5. AIGC时代的代码评审

github copilot X google在gerrit上的实践