CR的技巧

定位：工程实践方法论。Code Review 不是走流程，是质量防线的最后一环，也是团队知识流动的主要通道。这篇整理一套可复用的清单式评审方法。

为什么要有方法论

没有方法的 CR 常见两种失效形态：

• 橡皮图章式：Approve 点得飞快，问题全部漏到线上——CR 变成流程负担。
• 细节纠缠式：在命名和格式上吵三页评论，却没人看并发和事务边界——抓了芝麻丢了西瓜。

方法论解决的就是看什么（问题类别）、按什么优先级看（分级评审）、怎么表达（评论措辞） 三件事。

分级评审：先定级，再看代码

拿到一个 MR/PR，第一件事不是逐行看，而是判断这个变更的风险等级，再决定评审深度。

变更分级

等级	特征	评审要求
P0 高危	涉及钱/库存/权限/数据迁移/公共接口签名变更/线程模型变更	逐行精读 + 至少两人评审 + 必须有测试覆盖证明
P1 常规	业务逻辑变更、新增接口（非公共）、SQL 变更	完整过一遍逻辑分支 + 看测试
P2 低危	配置文案、日志、注释、纯重构（有测试兜底）	抽查 + 依赖 CI

评论分级

评审意见本身也要分级，让作者一眼知道哪些必须改、哪些可以下个迭代再说：

• [blocker] 必须修改才能合入：bug、并发安全、数据不一致风险、安全漏洞。
• [major] 强烈建议：边界缺失、异常分支未处理、性能隐患（循环内查库）。
• [minor] 建议：命名、方法拆分、魔法值提取。
• [nit] 吹毛求疵：个人风格偏好，作者可拒绝。

不分级的评论会让作者把所有意见当成"必须改"，review 变成拉锯战。或者把所有意见当成"随意"，blocker 被忽略。

常见问题类别：按层扫

按固定的类别顺序扫描，避免"想到哪看到哪"。以下八类覆盖了绝大多数线上事故的代码级根因。

1）正确性与边界

• 空值：参数、RPC 返回、Map.get、Optional 使用是否防住 NPE。
• 边界：0、1、负数、超长字符串、空集合、null 集合，是否都有明确分支。
• 越界：数组/下标计算、subList 的 fromIndex/toIndex。
• 溢出：int 累加是否会溢出（如 int 毫秒时间戳、数量统计）、除零。

2）并发安全

• 共享可变状态：成员变量、static 变量、缓存 Map 是否被并发读写。
• 非线程安全容器：HashMap/SimpleDateFormat/ArrayList 是否出现在多线程路径。
• 复合操作：check-then-act（先判断后执行）是否有原子性保证。
• 锁：锁粒度、锁顺序（死锁）、是否锁住了远程调用（长事务持锁）。
• 双重检查锁的 volatile、ThreadLocal 的 remove。

3）事务与一致性

• 事务边界：@Transactional 是否生效（自调用、非 public 方法、异常类型不匹配导致回滚失效）。
• 跨服务/跨库操作是否有补偿或消息最终一致方案。
• 先提交事务再发消息，还是先发消息——顺序错了就是不一致。
• 事务内是否有 RPC/文件 IO（拉长事务持锁时间）。

4）资源与泄漏

• 流/连接/线程池：try-with-resources、finally 释放。
• 线程创建：是否有 new Thread( 裸创建（应使用线程池）。
• 线程池参数：队列类型与容量、拒绝策略是否是有意的。
• 内存：大对象生命周期、静态集合只增不减（内存泄漏经典形态）。

5）性能

• 循环内查库/RPC：N+1 问题，最常见的性能杀手。
• 大集合操作：全量加载后内存过滤，应下推到 DB；深分页。
• 每请求新建重对象：如每次 new 一个大缓冲区、重复解析配置。
• 日志：高频路径打印大对象、字符串拼接先于级别判断、级别是否合理。

6）安全

• SQL 注入：拼接 SQL、like 通配符转义。
• 越权：接口是否校验了资源归属（横向越权），不能只信前端传的 userId。
• 敏感信息：密码/密钥/手机号是否明文打印日志、是否进了 git。
• 反序列化：不可信来源的 JSON/XML 解析配置。

7）可读性与可维护性

• 命名：方法名是否说真话（getUserById 就别干别的）。
• 单一职责：一个方法缩进过深、一个类塞多个业务概念。
• 魔法值：裸写数字/字符串，缺少常量或枚举。
• 重复代码：三处以上相同逻辑应提取。

8）测试与可验证性

• 新逻辑有没有测试；测试断言是否真的断言了行为（不是只 assert 不抛异常）。
• 边界条件有没有对应用例。
• 测试是否依赖外部真实环境（应可重复、可本地跑）。

Checklist 模板

可以直接贴到团队规范里的一份合并前清单。

提交者自审清单（提 PR 前）

[ ] 自查 diff 已通读一遍（不是写完直接提）
[ ] 本地跑通全部单测 + 相关回归
[ ] 无调试输出残留 / 注释掉的大段死代码
[ ] 大改动已拆分为多个独立小 commit
[ ] PR 描述写清楚：做了什么、为什么、影响面、怎么验证
[ ] 涉及接口变更已同步接口文档 / 已通知调用方
[ ] 涉及配置变更已列出配置项与默认值

评审者清单（Review 时）

[ ] 先看 PR 描述与关联需求，理解"应该改什么"
[ ] 先看测试：测试覆盖了哪些场景，缺哪些
[ ] 正确性：空值 / 边界 / 异常分支逐个过了一遍
[ ] 并发：新引入的共享状态是否安全
[ ] 事务：事务边界与回滚条件正确
[ ] 资源：流、连接、线程池均有正确生命周期
[ ] 性能：无循环查库、无明显大对象滥用
[ ] 安全：无注入、无越权、无敏感信息泄漏
[ ] 兼容：对外签名未变；变了是否有版本策略
[ ] 每条评论都标了级别（blocker/major/minor/nit）

流程与效率

控制 diff 大小

• 单次评审的有效 diff 控制在不超过 400 行，超过就拆。超过这个量之后人的漏检率显著上升。
• 大重构拆成"先搬代码不改逻辑"与"再改逻辑"两个 PR，第一个几乎不用看，第二个集中火力。

评论措辞

• 对代码不对人：说"这个分支没处理 null"，不说"你怎么又没判空"。
• 提问优于断言："这里并发调用会不会重复创建？"——给对方解释设计意图的机会，也避免自己看错打脸。
• 讲清为什么："循环里每次查库，1 万条数据就是 1 万次 RPC，建议改成批量" 优于 "别在循环里查库"。
• blocker 必须写清风险后果和期望的修法，让作者不用二次确认就能改。

评审时效

• 团队约定响应 SLA（如 4 小时内首次响应）。评审卡住是发布流水线的最大隐形瓶颈。
• 高优分支可以开评审会面对面过，效率远高于评论区三页往返。

认知同步

• 每周/每迭代挑 1-2 个典型 MR 做复盘：漏了什么、为什么漏。CR 的价值有一半在知识扩散。

高频反例速查

• 只看改动行，不看上下文——if 条件改动只看那一行，漏掉整个调用链的影响。
• 测试文件直接跳过——测试自身的 bug、拷贝修改出来的假通过，都藏在这里。
• "先合了后面再优化"——"后面"永远不来，blocker 降级成 major 的口子一开就收不住。
• 评审者只提 minor 不提 major——格式全对、事务是错的，是最讽刺的评审结果。
• 自动 Approve 脚本 / 互挂 Approve——一旦形成惯例，CR 防线等于零。

一句话总结

先分级定深度，按类别扫描，评论带级别和理由，测试是评审的一部分而不是附件。 CR 的产出不是 "Approve" 这个动作，而是合入前被消灭掉的那批缺陷。

2026-10 依据主题重写（原文件为空壳）。