代码评审该看什么：一份可执行的清单

评审的目标

很多人把代码评审理解成「找错」，于是变成了挑刺比赛。它的真实目标有三个，按重要性排序：

1. 防止缺陷进入主干（尤其是逻辑错误和边界问题）
2. 传递知识（让至少两个人理解这段代码）
3. 统一风格（减少后续维护的认知成本）

注意「性能优化建议」不在前三位。评审阶段提出重构建议通常得不偿失，除非确实存在明显问题。

先看整体，再看细节

上来就逐行读 diff 是最低效的方式。正确的顺序是：

1. 读提交信息与描述 → 这段改动想解决什么问题
2. 看改动范围     → 涉及哪些文件、多大体量
3. 看核心逻辑     → 主要流程是否走得通
4. 看边界与异常   → 空值、超长、并发、失败路径
5. 看细节风格     → 命名、注释、格式

前两步花一分钟，能省下后面十分钟的困惑。如果看完描述还不明白改动意图，应该直接提问，而不是硬猜。

六个必查维度

一、正确性

• 逻辑是否覆盖了描述里说的场景
• 有没有「看起来对但边界错」的地方
• 条件判断的正负号、比较符是否写反

二、边界条件

这是 bug 最集中的地方，重点看四类：

三、错误处理

• 异常有没有被静默吞掉
• 失败时数据是否还保持一致
• 有没有把内部错误信息直接暴露给用户

四、安全

• 用户输入有没有做校验和转义
• SQL 是不是拼接出来的（应该用参数化查询）
• 敏感信息有没有被写进日志或返回给前端
• 权限校验是不是每个入口都有

安全问题的特点是平时看不出问题，出事就是大事，所以值得单独过一遍。

五、可读性

• 命名能否自解释
• 函数是否过长（超过一屏就该考虑拆分）
• 嵌套层级是否过深（超过三层就该提前返回）
• 有没有解释「为什么」的注释，而不是复述「做了什么」

六、可测试性

• 逻辑是否和外部依赖耦合太紧
• 有没有把难以测试的副作用混进纯计算里

三个容易被忽略的点

第一，数据库变更。 加了字段有没有配套的迁移文件？迁移能不能在已有数据上安全执行？加索引在大表上会不会锁表？

第二，兼容性。 接口返回结构变了，调用方有没有同步改？老数据能不能被新代码正确解析？

第三，可回滚。 如果上线后发现问题，能不能只回滚代码而不动数据？如果不能，说明这次改动需要更谨慎的发布方案。

怎么给反馈

同样一条意见，说法不同效果差很多。我的几条原则：

区分「必须改」和「建议」。 明确标注，否则作者不知道哪些可以商量：

【必须】这里没有校验 amount 为正数，负数会导致余额反向增加。
【建议】这个函数 80 行了，可以考虑按职责拆成两个，非阻塞。
【提问】这里的 3000 是超时时间吗？单位是毫秒还是秒？

对事不对人。 不说「你这里写错了」，而说「这里在 X 情况下会返回 Y，是不是和预期不符」。

解释理由。 只给结论的评审意见很难被接受。说清楚「为什么」，作者才能举一反三。

该夸就夸。 看到巧妙的设计或完善的边界处理，明确说出来。评审不该只有负面反馈。

评审的边界

有些事不该在评审里做：

• 不要要求作者按你的风格重写。 除非项目有明确规范，否则风格差异不值得阻塞
• 不要在评审里做需求变更。 那是另一个讨论，应该单独开
• 不要无限期挂起。 小改动当天给结论，大改动不超过一天

小结

代码评审的价值是在成本最低的时候发现问题——改一行代码比改一个线上事故便宜太多。

按「整体 → 核心逻辑 → 边界 → 细节」的顺序看，重点关注正确性、边界、安全和可读性四个维度，反馈时区分优先级并说明理由。