代码审查不是找茬，是守住合约边界

我观察过很多团队的 Review 流程，最常见的两种失效模式截然相反：一种是「走过场」，reviewer 扫几眼，给个 LGTM 就合并；另一种是「太挑剔」，每次都揪出十几条注释，样式、命名、缩进都要争，搞得提 PR 的人如临大敌。

两种都没抓到核心。Code Review 的本质是：在代码进入主干之前，确认它没有引入新的合约破坏——安全漏洞、数据丢失风险、或让下游依赖方行为不可预测的改动。

置信度过滤：只报告你真的确信的问题

这是我认为最被低估的一条原则。

不少 reviewer 会把「我觉得可以更好」和「这里有 bug」混在一起报告，导致真正重要的问题淹没在样式偏好里。我的做法是给自己设一条门槛：超过 80% 确信是真实问题才写出来，否则放进心里或者口头聊。

具体到执行：

• 风格偏好（缩进、引号风格）——跳过，让 linter 管
• 未变更文件里的旧问题——跳过，除非是 CRITICAL 安全漏洞
• 同类问题合并——「5 处缺少错误处理」是一条，不是 5 条

这个过滤机制的副作用是：reviewer 自己也会被逼着想清楚「我的依据是什么」，而不是凭感觉评价。

分级审查：CRITICAL 到 LOW，优先级决定合并决策

把审查发现分成四级，直接映射到合并建议：

CRITICAL（Block）：存在就不能合并。SQL 注入、硬编码密钥、认证绕过、日志泄露 Token/PII——这类问题一旦上线，修复代价是指数级的。

HIGH（Warning，建议修复）：函数超 50 行、深层嵌套超 4 层、Promise 没有错误处理、Vue 2 的响应式失效……这些不会立刻炸，但会让下一个接手的人付出沉重代价。

MEDIUM（建议）：性能问题，比如 SELECT *、循环里查数据库（N+1）、大列表没有虚拟滚动。可以合并，建议后续跟进。

LOW（备注）：魔法数字、命名模糊、TODO 没关联 Issue。记录在案，不影响合并决策。

最容易被跳过的一步：阅读上下文

大多数审查工具默认只展示 diff。这带来一个隐患：reviewer 孤立地看变更，不知道这个函数的调用方是谁，也不知道被改动的类在整个模块里扮演什么角色。

我的习惯是在审查前做一步「范围识别」：

1. 看变更文件，确认影响的功能域
2. 阅读完整文件，而不只是高亮的行——理解 imports、依赖和调用链
3. 如果改动碰了接口层（API 契约、数据库 schema），额外追溯下游消费方

这一步费时，但能捕捉到很多 diff 视图里看不出来的问题，比如「这个新增字段没有在所有列表查询里加过滤，会漏数据」。

技术栈特化：通用原则之外的额外清单

通用的安全/质量检查项是基础，但不同技术栈有自己的高频陷阱，值得单独列出来对照。

以 PHP 为例：请求参数直接用没经过 Validate 校验、多表操作没包事务、HTTP 请求没设超时——这三条是最容易在赶进度时被忽略的，也是事故概率最高的。Vue 2 里对应的是 $set() 遗漏导致响应式失效、beforeDestroy 没清监听器导致内存泄漏。

把这些技术栈特定的检查项维护成一份清单，和通用原则并列使用，比凭经验临场发挥要可靠得多。

一句话心法

Code Review 的目的不是让代码变完美，而是让危险的代码不进主干。