Engineering Leadership

代码审查的艺术：从对抗到协作

代码审查不是找茬游戏，而是知识传递和团队成长的机会。分享我 5 年来从'审得让人哭'到'审得让人笑'的心路历程，以及一套让代码审查成为团队加速器的方法论。

Ioodu · · Updated: Mar 2, 2026 · 18 min read
#Code Review #Team Culture #Engineering Practices #Knowledge Sharing

那场让我醒悟的冲突

“你的代码写得有问题，这里要用策略模式。”

“我觉得现在挺好的，没必要改。”

“这是最佳实践，不改的话我不能 approve。”

“那你来改？”

那是我作为 Tech Lead 的第二年，我和团队里一位资深工程师在 PR 里吵了起来。最后他摔门而去，PR 挂了三天。

那一刻我意识到：我搞错了代码审查的目的。

我以为代码审查是确保代码”正确”。但真正的目的应该是：

  • 知识共享：让团队了解新代码
  • 学习成长：帮助彼此写得更好
  • 风险防控：发现潜在问题
  • 团队共识：建立共同的标准

今天这篇文章，分享我 5 年来从”审得让人哭”到”审得让人笑”的心路历程。

代码审查的三个层次

第一层：找茬式（Level 1: Nitpicking）

典型特征：

  • 关注缩进、命名、空行
  • “这里少了一个空行”
  • “这个变量名应该用 camelCase”

问题：

  • 浪费时间在自动化能解决的问题上
  • 让作者感到被针对
  • 忽略了架构和设计问题

我曾经是这样的审查者。

花了 30 分钟在一个 PR 里挑了 50 个格式问题，作者改了 2 小时。后来我们引入了 ESLint + Prettier，这些评论全都不需要了。

改进：

  • 用自动化工具解决格式问题
  • 只关注工具无法检查的内容
  • 建立团队代码规范，自动执行

第二层：纠错式（Level 2: Correcting）

典型特征：

  • 指出逻辑错误和潜在 bug
  • “这里可能有空指针异常”
  • “这个算法复杂度是 O(n²)，会超时”

进步：开始关注真正重要的问题

但仍然有问题：

  • 语气直接，像下命令
  • “应该”这个词频繁出现
  • 缺乏解释，只是指出问题

典型评论：

❌ 这样写不对，应该用 Promise.all

改进后的评论：

💡 这里用 Promise.all 并行执行可能会更快。

当前是顺序执行：
const a = await fetchA();
const b = await fetchB(); // 等待 fetchA 完成

可以改为并行：
const [a, b] = await Promise.all([fetchA(), fetchB()]);

这样能减少 ~200ms 的等待时间。

第三层：协作式（Level 3: Collaborating）

典型特征：

  • 提问多于下结论
  • 分享知识而不是显示权威
  • 承认自己的不确定性

评论风格：

🤔 我在想，如果用户同时提交两个请求，这里会不会有竞态条件？

我想到两种方案：
1. 数据库唯一约束
2. 分布式锁

你觉得哪种更适合我们的场景？

区别：

  • 不是”你错了”，而是”我们一起看看”
  • 不是”必须改”，而是”考虑一下”
  • 不是”我是对的”，而是”这样可能更好”

代码审查的语言艺术

用词的转变

❌ 不要说✅ 尝试说
这里错了这里可能有风险
应该用 XX 会不会更适合？
不符合规范我们的规范建议…
重写吧有没有考虑过…
你为什么…我理解你的做法，同时想讨论…
这不行我担心这会在 XX 场景下出问题

提问的力量

下结论：

这个函数太长了，拆成三个。

提问题：

这个函数有 80 行，在做三件事：验证、转换、存储。

我在想是不是可以拆成：
- validateInput()
- transformData()
- saveToDB()

这样每个函数只有一个职责，也更容易测试。你觉得呢？

承认不确定性

我对这块业务不太熟悉，可能理解有误。

从代码看，当 status 是 'pending' 时不会触发通知，这是预期的行为吗？

如果是，加个注释说明可能会帮助其他读者。如果不是，可能需要检查一下状态机的逻辑。

效果：

  • 降低对抗性
  • 鼓励作者解释思路
  • 避免自己犯错时的尴尬

代码审查的框架：EIQ

我总结了一个简单的框架：EIQ (Essential - Important - Question)

Essential（必须改）

标准：会导致 bug、安全漏洞、性能灾难

审查标记：🔴

例子：

  • SQL 注入漏洞
  • 未处理 promise rejection
  • 无限循环风险
  • 内存泄漏

语气：明确但尊重

🔴 这里需要修复：用户输入直接拼接到 SQL 中，存在 SQL 注入风险。

建议使用参数化查询：
db.query('SELECT * FROM users WHERE id = ?', [userId])

参考：[OWASP SQL Injection Prevention](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)

Important（建议改）

标准：影响可维护性、性能、可读性，但不会立即导致问题

审查标记：🟡

例子：

  • 重复代码
  • 复杂度过高的函数
  • 缺少错误处理
  • 命名不清晰

语气：建议性的

🟡 这段逻辑在 auth.ts 里也出现过，考虑提取到共享的 validateToken() 函数？

好处：
- 避免重复代码
- 统一验证逻辑
- 更容易测试

不急，可以在下个 PR 处理。

Question（讨论）

标准：不理解、想学习、寻求不同方案

审查标记：🟢

例子：

  • 设计选择的原因
  • 算法选择
  • 库的选择

语气：开放的

🟢 我注意到你选择了 LRU 缓存而不是 TTL，能分享一下考虑吗？

我之前用 TTL 比较多，因为实现简单。LRU 在内存管理上确实更好，但想知道在实际使用中效果如何？

代码审查的流程优化

1. 小步快跑

反模式：

  • 一个 PR 改了 50 个文件，2000 行代码
  • 审查者花了 3 小时，眼都花了
  • 作者等了一周才合并

最佳实践：

  • 每个 PR < 400 行代码
  • 每个 PR 只做一件事
  • 复杂的重构分多个 PR

2. 自审清单

作者提交前自查：

## PR 自查清单

### 功能
- [ ] 功能实现完整
- [ ] 边界条件处理
- [ ] 错误场景测试

### 代码质量
- [ ] 测试覆盖率 > 80%
- [ ] 没有重复代码
- [ ] 命名清晰易懂

### 文档
- [ ] 复杂逻辑有注释
- [ ] API 变更更新了文档
- [ ] CHANGELOG 已更新

3. 审查时间盒

规则：

  • 每个 PR 第一次审查 < 4 小时
  • 每个 PR 审查时间 < 30 分钟
  • 如果 30 分钟审不完，PR 可能太大了

4. 轮替审查

不要：总是同一个人审查某人的代码

要：轮替配对，让知识在团队中流动

好处：

  • 避免单点依赖
  • 新人快速成长
  • 减少”这是 XX 的代码，我不敢审”

处理分歧

当作者不同意

场景：你认为应该改，作者认为不需要

做法：

  1. 先听：让作者解释他的考虑
  2. 再讲：说明你的担忧和理由
  3. 寻找中间方案：也许有第三选项
  4. 升级：如果无法达成一致，找 Tech Lead 或团队讨论

话术：

我理解你的考虑，性能确实很重要。

我担心的是在并发场景下，这个方案可能会丢失数据。

要不我们：
1. 先加个监控，看看实际并发量
2. 如果并发不高，保持当前方案
3. 如果并发高，再考虑加锁

你觉得这个 plan 怎么样？

当审查被拖延

场景：PR 挂了 3 天没人审

做法：

  1. 提醒：@channel “这个 PR 等待审查，有 2 天了”
  2. 分解：如果太大，分解成小的
  3. 紧急通道：真正紧急的，找人当面审查

团队规则：

PR 审查 SLA：
- 普通 PR：24 小时内
- 紧急 PR：4 小时内
- 周末不算

如果超时，可以合并（需要 Tech Lead 批准）

代码审查的工具链

GitHub + 浏览器插件

GitHub Copilot for PRs：

  • AI 生成 PR 描述
  • AI 代码解释
  • AI 审查建议

CodeRabbit、PR-Agent：

  • 自动代码审查
  • 找出潜在问题
  • 生成测试建议

本地工具

gh cli：

# 列出等待我审查的 PR
gh pr list --review-requested=@me

# 快速 checkout PR 分支
gh pr checkout 123

Review Dog：

  • 本地运行 linter，在提交前发现问题

培养审查文化

领导示范

Tech Lead 的 PR 也要被审查，而且要：

  • 公开接受批评：“你说得对，我改”
  • 承认错误：“这里我确实没考虑周全”
  • 感谢审查：“感谢指出，学到了”

审查回顾

每月一次 15 分钟的站会：

  • 这个月的审查质量如何？
  • 有没有反复出现的问题？
  • 流程可以怎么优化？

正向反馈

不只是找问题，也要点赞：

🎉 这个 error handling 写得真好，每个边界情况都考虑到了！
💡 TIL: 原来可以用解构赋值默认值，学习了！

总结：代码审查的十条原则

  1. 审查代码，不审查人：对事不对人
  2. 提问多于下结论：用”是不是”代替”应该”
  3. 解释你的理由：帮助作者理解为什么
  4. 承认不确定性：“我可能理解错了”
  5. 区分必须和建议：🔴🟡🟢 标记清楚
  6. 保持小 PR：审查者也是人，会疲劳
  7. 及时响应：尊重彼此的时间
  8. 轮替配对：知识共享，避免单点
  9. 给予肯定：好的代码要表扬
  10. 持续改进：定期回顾审查流程

代码审查的最高境界：

作者收到审查意见后，不是感到被批评，而是感到”学到了新东西”。

当代码审查成为团队的学习时刻，而不是对抗时刻，你就成功了。

参考资源

你经历过哪些难忘的代码审查？欢迎分享你的故事。

---

评论