四个人的团队，怎么做代码评审
技术

四个人的团队，怎么做代码评审

周三下午三点，仓库里躺着一个开了三天的 PR。diff 一千二百多行，标题写着「一些优化」。作者自己在群里问：要不我关掉重开一个？

那天我们四个人花了两个小时，把它拆成四个小的，又花了一个下午评审、修改、合并。真正需要讨论的只有两处：一个并发写的位置，一个空值判断。剩下的八百多行，是顺手改的命名和格式化。

那之后我们改了点东西。

先承认我们请不起「评审流程」

我们一共四个人，两个后端、一个前端，加一个半职的产品兼测试。没有专职 reviewer，没有 QA 卡点，也没有人愿意在周五晚上看别人代码。

一开始我们照抄了一套大厂流程：两人 approve、CODEOWNERS、所有检查必须绿、评审不通过不能合并。结果不到两周，所有人都学会了「先点 approve 再慢慢看」，或者干脆去找那个最好说话的人。CODEOWNERS 那个文件我配完之后就没再打开过：人太少，谁都能改任何目录，写了等于没写。

后来我们把这套东西拆了，只留三条：

  1. 每个 PR 必须有人看过，一个人就够；
  2. 涉及数据库迁移和支付的，两个人看；
  3. PR 描述里必须写清楚「怎么验证」。

其余都是建议，不是规则。规则越少，越有人当真。

机器能判的，不给人判

我最不想在评审里看到的两类评论，是「这里少个空格」和「这个函数名不好」。前者机器能做，后者机器做不了，也很难靠评审达成一致。

我们的 CI 大概就这几条，PR 一推就自动跑：

YAML10 行
1
2
3
4
5
6
7
8
9
10
# .github/workflows/pr.yml 的主要部分
jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - run: npm ci
      - run: npm run lint -- --max-warnings=0
      - run: npx tsc --noEmit
      - run: npm test
      - run: npx prettier --check .

prettier 和 lint 定死格式，tsc 挡住类型问题，单测挡住明显的逻辑错。这几条不通过，PR 页面根本轮不到人说话。

覆盖率我们没设阈值。以前设过 80%，结果冒出一堆 expect(fn).toBeDefined() 这样的断言，那才是真浪费。现在只是在 PR 页面把覆盖率变化显示出来，降了会问一句为什么。

这一层的收益其实不是「省时间」。以前一个 PR 我能写二十条评论，十八条关于格式；现在只有两三条，但每条都值得停下来聊两句。

响应要快，PR 要小

小团队最常见的状态不是「评审太严」，是「没人理」。

我自己就干过：早上看到通知，想着等会儿空下来仔细看，然后一到下班就忘了。作者那边就卡着，不知道是该继续改还是该等，不敢合并，也不敢开下一个。

所以我们约定了一个很简单的东西：工作日上午 11 点和下午 4 点，值班的人扫一眼待评审列表。值班轮流，一周一换。

值班人的义务不是「看完」，是「在四小时内给出第一次响应」。响应可以是提了意见，可以直接 approve，也可以是一句「我今天排满了，明天上午看」，这句也算，至少作者知道要等多久。

这条规则唯一的作用，是消灭「发出去了、没有回音」的状态。人得不到反馈就会开始猜，猜久了就会绕开流程：直接往主干推，或者干脆不提 PR 了。

跟响应速度配套的是颗粒度。一千二百行那个 PR，复盘出来的结论是：它不是一个 PR，是六个。

现在的习惯是动手之前先问自己，这个改动能不能用一句话说完。说不完就拆。顺手看到的重构、格式化、改错别字，单独开一个 PR，十分钟能合掉的那种。

大致上，diff 在 300 行以内，评审质量还行；超过 500 行开始敷衍；超过 1000 行基本就是点个通过。这个数字没有严格依据，是我们自己感觉到的拐点。所以现在有人在群里说「我这个 PR 有点大」，其他人的第一反应是「拆一下」，而不是「发来看看」。

但拆 PR 的成本不低：要调分支、要处理依赖顺序，有时候两个 PR 必须一起上。所以我不觉得这是无条件的对。修线上问题的时候，一个大 PR 直接上完全没问题，事后补个说明就行。规则是给日常迭代用的，不是给救火用的。

意见怎么写，以及新人的例外

评审最容易伤人的地方是语气。同样一句话，「这里不对」和「如果 user 是 null 会怎样」，后者不会让人想反驳。

我们内部约定了几种前缀，写在 PR 模板里：

  • block: 这个必须改，不改不能合；
  • q: 我没看懂，你能解释一下吗；
  • nit: 小问题，可改可不改；
  • note: 留给未来的自己看，不用回。

加了前缀之后有个意外的好处：作者能一眼看出哪些是硬要求，不用每条都去猜。

另一个我坚持的习惯是，评审完写一句「我看了哪部分」。比如「看了路由和鉴权，SQL 那部分没细看」。这话对自己是提醒，你确实没看完；对作者是信息，哪块还没人看过。比一个孤零零的 LGTM 有用得多。

还有一条我吃过亏才学会的：不要把「我觉得应该换个设计」写在一行代码的评论里。那是另一个层级的问题，写在 PR 的总评论里，或者干脆开个会聊十五分钟。挂在某一行上，作者只会觉得你在挑刺。

最后说个例外。上个月来一个实习生，第一个 PR 我按流程评了，写了七八条。他改完了，第二天又犯了同样的错。后来我改成第一次提 PR 之前先结对看半小时他的代码，边看边讲。那半小时之后，他的 PR 质量明显上来了。

评审对熟练的人是效率工具，对新人不是。新人缺的不是「指出错误」，是「知道为什么这样写」，这两件事的解法不一样，只是小团队里我们总想用一套流程解决所有问题。

下周我们打算在周会的小面板上加一行「PR 从开到第一次响应的小时数中位数」。如果哪一周这个数超过 12 小时，就把当周的迭代内容砍掉一件。这个还没跑满一个月，会不会变成新的形式主义，我不好说。

关于本文

由 Lu's Blog 撰写，采用 CC BY-NC 4.0 许可协议。

#代码评审 #小团队 #工程效率

评论