代码审查的艺术：让 review 不再是一场互相折磨
提起 code review，很多人的第一反应是”又要被挑刺了”，或者是”又要花半小时给别人挑刺了”。于是它慢慢变成了一种仪式感：点开 diff，扫一眼，回一句 LGTM，完事。
这挺可惜的。代码审查本应是团队里性价比最高的一件事——它能在 bug 上线之前拦住 bug，能在知识只存在于一个人脑子里的时候把它扩散出去，还能让新人快速理解项目的套路。
问题从来不在”要不要 review”，而在”怎么 review”。做得好，它是给项目上保险；做得差，它就成了互相消耗。今天聊聊我踩过坑之后，觉得真正管用的几个做法。
先搞清楚：review 到底在找什么
很多人 review 时下意识把自己当成”捉虫的”，盯着逻辑找 bug。这没错，但不是全部。我一般按四个层次往下看，从宏观到微观：
- 架构和设计：这个改动放到系统的整体里，位置对不对？是不是在正确的地方做了正确的事？有没有引入不应该出现的耦合？
- 正确性：逻辑对不对，边界条件考虑全了没有，异常路径处理了吗。
- 可维护性：命名清不清楚，有没有重复代码，几个月的自己还能不能看懂。
- 规范：风格、约定、还有那些”这个项目里约定俗成”的东西。
前两个层次比后两个重要得多。一个命名一般但逻辑正确的 PR,和一个命名漂亮但藏了 bug 的 PR,前者价值远高于后者。很多人本末倒置，把大量精力花在”这行缩进不对”上，反而放过了真正的设计问题。
别在一百行里找那一行的茬
这是我最想强调的一条。review 不是”找茬比赛”，改动的作者也不是你的对手。目标只有一个：让这次改动更好地上线。
挑毛病的语气很重要。同样是”这里应该用 Map 而不是遍历数组”，可以说：
这地方遍历数组每次查询都要 O(n)，数据量一大就慢了，
要不要考虑用 Map？如果数据量始终很小，那现在这样也没问题。
也可以说：
怎么又用数组遍历？早说了用 Map。
两句话表达的是同一个技术意见，但前一种给了理由、给了边界、把决定权交还给了作者；后一种只留下情绪，下次作者可能连 review 都懒得认真看。
我给自己定的规矩是：意见要具体到”为什么”，留出”可以不改”的余地，把主观偏好和技术事实分清楚。“我喜欢驼峰多于下划线”是偏好，”这个写法在并发下会丢数据”是事实，两者的分量天差地别。
小 PR 是好 PR
这条看着跟 review 没关系，其实关系最大。
一个改了两百行、横跨三个模块、还顺手重构了一堆东西的 PR,谁看都头疼。reviewer 根本没法建立完整的上下文，最后要么草草通过，要么在无关紧要的地方东一榔头西一棒子。
反过来，一个只做一件事、diff 控制在百行以内的 PR,review 起来又快又准。
怎么让 PR 变小？几个实际操作：
- 一次只解决一个问题，别把”修 bug”和”重构”混在一个 PR 里。
- 重构和功能变更分开提：先提交纯粹的重构（行为不变），再提交功能改动，review 起来干净得多。
- 大的功能拆成一个个可合并的小步骤，每一步都能独立审查、独立回滚。
坏例子：一个 PR 里改了登录逻辑 + 重构了用户模块 + 升级了依赖
好例子：
PR #1：升级依赖（无行为变更）
PR #2：重构用户模块（纯搬代码，行为不变）
PR #3：实现新的登录逻辑（真正的功能改动）
拆完之后你会发现，review 变快还在其次，更重要的是出问题的时候能精确定位——回滚 PR #3 就行，不用连前面的重构一起受牵连。
善用自动化，把机械劳动外包出去
review 里有很多”不需要人脑”的部分：格式对不对、有没有明显未使用的变量、测试过没过、代码风格是否统一。这些事硬要人去盯，纯粹是浪费。
把 lint、格式化、单元测试、类型检查都接进 CI，让机器先把那层”低级错误”捞出来。等 PR 到人手里的时候，剩下的才是真正值得人看的：设计、逻辑、可维护性。
# 一个最小可用的 CI 检查，把人从机械 review 里解放出来
name: PR checks
on: [pull_request]
jobs:
check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 20
- run: npm ci
- run: npm run lint # 风格和潜在错误
- run: npm run test # 单元测试
- run: npx tsc --noEmit # 类型检查
有了这套东西，reviewer 就不用在格式这种鸡毛蒜皮上耗神了，可以把注意力留给真正的问题。
对事不对人，也要对”响应速度”负责
review 还有一层容易被忽略的成本：等待。一个 PR 卡在 review 里三天，作者只好切换到别的任务，上下文就断了，再捡回来又要重新进入状态。这种隐性损耗，远比很多人以为的大。
我的习惯是：
- 当天提交的 PR 当天给反馈，哪怕只是”我明天细看”。
- 优先 review 别人的 PR，而不是先写自己的新代码——团队里很多人都在等你，你的新功能反而可以等。
- 小 PR 当场看完；大的约个时间一起过一遍关键设计，别在评论里来回拉锯。
审查者快一点，作者少一点反复，整个团队的吞吐量才能上来。
收尾：一条好 PR 描述胜过十句微信追问
作者这边能做的最有用的一件事，是把 PR 描述写好。reviewer 打开 diff 之前，先知道你要干什么、为什么这么干、影响范围多大，能省掉大量来回确认。
一条合格的 PR 描述通常包含：
- 动机：为什么要做这个改动，解决了什么问题。
- 方案：怎么做的高层说明，有哪些关键决策。
- 影响面：会不会影响其他模块、接口有没有变化、有没有迁移步骤。
- 自测情况：本地怎么验证的，跑过哪些用例。
## 背景
登录时未做频率限制，有被暴力尝试的风险。
## 做法
登录接口加基于 IP + 账号的限流，失败 5 次后锁定 15 分钟，
配置通过环境变量暴露，默认开启。
## 影响面
- 新增限流中间件，现有登录接口行为不变
- 需要在部署配置里加 RATE_LIMIT_MAX 等变量
## 自测
单测通过；本地模拟连续失败验证锁定达标。
reviewer 读完这段，再看 diff,心里就有谱了，问的都是真问题，而不是”这个 PR 是干嘛的”。
小结
代码审查说穿了，就是一群人对交付质量共同负责。它好的时候，团队会形成一种正向循环：认真写、认真看、互相学到东西；坏的时候，就退化成走过场，谁都觉得浪费时间。
落到日常，无非这几条：
- 分清主次，把精力放在设计和正确性上，而不是格式。
- 意见要具体、给理由、留余地，别把 review 当找茬。
- 想尽办法把 PR 做小，小到能快速审、准确定位、轻松回滚。
- 用 CI 把机械检查外包给机器。
- 作者写好描述，审查者当天响应。
这些事都不难，贵在坚持。当 review 真正转起来的那一刻，你会发现团队的代码质量和协作体验，是同步往上走的。