12.代码审查文化建设

# 12.代码审查文化建设

本篇定位：外科第三篇 · 康复训练——CR 卡点、评审礼仪、自动化门禁。

剧情节点：Day 79-85——重构已成，但沈总提出终极追问："下一位小李接手，会不会又把它写烂？"答案在 CR 文化里。

本篇病人：P-001 全系统 · 通过 CR 文化把品质从"个人英雄"变成"团队默认"。

承接经典：Google《Engineering Practices Documentation》· Code Review 章 / SmartBear《Best Kept Secrets of Peer Code Review》/ Karl Wiegers《Peer Reviews in Software》。

本篇病案编号范围：R01-R08（代码审查类全套）

# 1. 急诊病例

# 1.1 点头惨案回放

Day 79，08:52，OrderMonolith 生产告警群：

[P0] 优惠券活动首日崩溃，5 分钟内下单成功率跌至 12%，客服工单涌入 800+ 单。

老陈接到电话时正在喝豆浆，一口全喷在了键盘上。他打开 Sentry，异常栈指向新上线的优惠券模块 CouponVerifier#apply：

public BigDecimal apply(Order order, Coupon coupon) {
    BigDecimal discount = coupon.getAmount();
    // Bug：没有校验优惠券是否过期，也没有校验金额是否为负
    return order.getTotal().subtract(discount);
}

问题极其低级——过期券没拦、金额没做非负校验，被羊毛党一夜之间"下单 −1 元"薅走 47 万元优惠金。

老陈翻出该 PR：#3841，提交者小张（新入职 2 周），代码 42 行。

审批记录如下：

审批人	时间点	评论
小李（同事）	Day 76 · 15:22	`LGTM 👍`
老王（Tech Lead）	Day 76 · 17:03	`+1`
CI 状态	Day 76 · 17:04	✅ 通过（覆盖率 82%，无 Sonar 阻断）

没有一个人真正读过这段代码。沈总把这份审批记录截图打印出来，钉在办公室大门上：

"我们花了两个月做重构、写测试、量化技术债——却在一个下午被两句 'lgtm' 全部报废。代码审查不是签字，是最后一道防线。"

老陈盯着截图，心里有一个更冷的问题：这样的 PR，这两个月过去多少个？

# 1.2 团队 CR 现状调研

Day 79 下午，老陈拉小李做了一次紧急 PR 统计（过去 30 天）：

指标	数值	健康水位	判定
PR 总数	156	—	—
仅一句评论（lgtm/+1/👍）通过	136 个（87.2%）	< 20%	重度失灵
PR 平均 diff 行数	1247 行	< 400	超标 3 倍
PR 平均评审时长	6.2 分钟	15-45 分钟	过快
提出改进建议的 PR 数	20	> 50%	仅 12.8%
PR 从提交到合并平均等待	2.4 天	< 1 天	略慢
有 reviewer 打回重做的 PR	3 个	10-20%	几乎没有

老陈画完这张表，小李脸都白了：

"我们平均 6 分钟看 1247 行代码，还不如没看。这不叫审查，这叫过场。"

老陈把结论写在白板上：

CR 名存实亡，团队实际是"个人英雄式提交 + 集体默认放行"——品质完全依赖提交者的自觉。

# 1.3 本篇待答疑问

老陈拿起马克笔，在白板中央写下 6 个问号：

为什么大家不认真 CR？（能力？时间？还是文化？）
一份好 PR 应该长什么样？ 拆多小？怎么写描述？
审查者应该看什么？不该看什么？ 有没有卡点清单？
如何给出让人愿意接受的意见？ 语气、颗粒度、优先级怎么定？
哪些审查工作能被自动化替代？（linter、Sonar、pre-commit、CI 门禁）
CR 文化怎么从"个人努力"变成"组织默认"？ 有没有可复制的制度？

这 6 个问号，就是本篇的完整地图。

**"下集提要（本节）：**先看病理诊断——CR 失灵的五种形态，你会发现 lgtm 只是其中最轻的一种。"

# 1.4 一句话回顾

一起因 CR 走过场引发的事故，让团队第一次意识到'CR 是文化，不是形式'。

# 1.5 核心要点

'lgtm'式 CR 是最贵的形式主义
CR 是团队学习最快的通道
CR 文化的下限决定项目质量的下限

# 1.6 带走清单

[ ] 禁止空评论 CR
[ ] CR 意见必须引用规则或编号
[ ] 每周复盘 CR 命中率

# 2. 病理诊断

# 2.1 CR 失灵五态

老陈翻遍 SmartBear《Best Kept Secrets of Peer Code Review》与 Google Engineering Practices 后，把 CR 失灵归纳为五种典型形态（F1-F5）：

形态编号	名称	症状	危害等级
F1	橡皮图章型 lgtm	只看标题、直接点 approve	★★★★★
F2	超大 PR 型	一次 2000+ 行、无法阅读	★★★★☆
F3	马拉松型	挂 7 天没人看，作者被迫再改冲突	★★★☆☆
F4	拳击场型	意见带情绪、伤自尊、演变成人身冲突	★★★★★
F5	形式主义型	只挑格式空格、放行核心逻辑漏洞	★★★★☆

每一种形态都有共同根因：CR 缺乏"什么该看、什么不该看"的共识，也缺乏"多久要看完、卡不卡合并"的约束。

Google Engineering Practices 用一句话概括：

"Code review is not about finding bugs; it's about maintaining code health."（Code review 不是为了抓 Bug，是为了守住代码健康度。）

而这句话我们团队从没真正内化过。

# 2.2 R 系列病案登记

小李按写作模板的病案卡片格式，登记了本篇的 8 个病案：

病案卡 R01：橡皮图章 lgtm

症状：审批仅一句 lgtm，无任何具体意见
危险等级：★★★★★
首次出现：CouponVerifier PR #3841
影响：漏过所有逻辑级缺陷，形同没审
治疗手段：CR 卡点清单 + PR 描述模板 + 强制文字性意见

病案卡 R02：超大 PR

症状：一次合并 > 400 行 diff，reviewer 无法阅读
危险等级：★★★★☆
首次出现：小张 CouponModule 1247 行合并
影响：审查者疲劳，倾向直接 approve
治疗手段：拆 PR + 单 PR 上限 + Stacked PR 工具（如 Graphite）

病案卡 R03：马拉松 PR

症状：PR 挂 > 3 天无人看
危险等级：★★★☆☆
首次出现：老王 API 网关 PR，挂了 11 天
影响：作者被迫反复解冲突，士气受挫
治疗手段：SLA 24h 首评 + 每日 PR 站会

病案卡 R04：拳击场评审

症状：意见带情绪，如 "这也叫代码？"
危险等级：★★★★★
首次出现：小张自尊心受挫，写完 PR 主动离职
影响：伤人 + 减损未来提交意愿
治疗手段：评审礼仪三原则 + 句式模板（nit/consider/must）

病案卡 R05：形式主义 CR

症状：只挑格式、忽视逻辑
危险等级：★★★★☆
首次出现：某 PR 收到 30 条空格意见但漏掉 NPE
影响：品质错觉 + 浪费精力
治疗手段：格式类交给 formatter/linter，人只审设计与逻辑

病案卡 R06：无 PR 描述

症状：PR 只有标题，没有 Why / How / Test 说明
危险等级：★★★☆☆
首次出现：60% PR 描述留空
影响：审查者要先花时间"猜意图"
治疗手段：PR 模板强制填写 Why/How/Testing/Risk

病案卡 R07：无门禁

症状：本地测试不跑、CI 挂了也能合并
危险等级：★★★★☆
首次出现：某绕过 CI 的 hotfix 直合入 main
影响：品质门失守，问题下沉到生产
治疗手段：pre-commit + branch protection + required checks

病案卡 R08：CR 无度量

症状：CR 通过率、评审时长、bug逃逸率无监测
危险等级：★★★☆☆
首次出现：老陈今日才第一次统计 87% lgtm 率
影响：CR 质量不可见 → 不可改进
治疗手段：Gerrit/GitHub GraphQL API 抓 CR 度量 → 团队看板

# 2.3 一句话回顾

R01-R08 八条 CR 病案，把'感觉 CR 不认真'翻译成'具体触发了哪条'。

# 2.4 核心要点

CR 反模式与代码反模式同等重要
编号让 CR 意见更精准
编号是团队沟通的最短路径

# 2.5 带走清单

[ ] R01-R08 表进 CR 模板
[ ] 团队 CR 至少覆盖 5 条 R 系列
[ ] 编号命中率纳入季度指标

# 3. 病因追溯

# 3.1 CR 不用心

老陈把小李、老王、小张三人叫进会议室，问同一个问题："为什么给别人 CR 时不认真？"

回答如出一辙但角度不同：

小李（初级）："我怕看不懂，看不出问题反而暴露自己菜。所以直接 lgtm 最安全。"
老王（资深）："我一天 20 个 PR 要看，看不过来。而且我下面还有 3 个人天天堵门口要评审。"
小张（新人）："我不知道该看什么。他们说 'lgtm 就行'，我就 lgtm。"

老陈把这三种心态归纳为 CR 失灵三大心理根因：

根因	心态	破解方向
R-Fear（怕丢脸）	怕看不懂显得菜	提供卡点清单，让"看什么"标准化
R-Overload（超负荷）	太多 PR 看不过来	PR 限流 + 拆分文化 + SLA
R-Ignorance（不懂标准）	不知道该看什么	写作培训 + 好 PR 模板 + 反例集

关键洞察：CR 失灵从来不是态度问题，是系统设计问题。

Karl Wiegers 在《Peer Reviews in Software》里说：

"If you rely on individual heroism, you're building a house of cards. Systematic reviews build systematic quality."（依赖个人英雄主义就是纸牌屋。系统化的评审才能带来系统化的品质。）

# 3.2 卡点礼仪双缺

老陈把 CR 失败拆成两条独立的失败链，各自都能独立杀死 CR 文化：

失败链 A：卡点缺失（技术侧）

无卡点清单 → 审查者看不明白该看什么
    ↓
无 PR 模板 → 提交者不写 Why / How / Test
    ↓
无门禁自动化 → 格式/覆盖率/静态问题占用人力
    ↓
人力全被浪费在低价值检查上
    ↓
真正的逻辑缺陷通过率 100%

失败链 B：礼仪缺失（人文侧）

无评审礼仪 → 意见带情绪
    ↓
提交者受挫 → 抵触 CR
    ↓
审查者害怕冲突 → 只给 lgtm
    ↓
CR 变成"礼貌的空转"
    ↓
文化死亡

两条链任何一条崩了，CR 都会失效。我们团队两条都崩了。

**"下集提要（本节）：**病因清楚了。下一节看治疗方案总纲——卡点 + 礼仪 + 门禁三合一。"

# 3.3 一句话回顾

CR 不用心的根因，是卡点缺失 + 礼仪缺失的双重失败。

# 3.4 核心要点

无卡点则 CR 变形式
无礼仪则 CR 变吵架
文化建设先卡点再礼仪

# 3.5 带走清单

[ ] 制定 CR 硬卡点清单
[ ] 写下团队 CR 礼仪红线
[ ] CR 冲突走升级机制

# 4. 治疗方案总纲

# 4.1 CR 三层地图

老陈画了一张"CR 三层地图"，贴在白板正中央：

┌────────────────────────────────────────────────┐
│ Layer 3: 组织层（文化 · 度量 · SLA）           │
│    ├─ CR 度量看板：通过率 / 首评 SLA / 逃逸率  │
│    ├─ 每季度评审礼仪培训                       │
│    └─ Champion + 结对 CR 机制                  │
├────────────────────────────────────────────────┤
│ Layer 2: 团队层（卡点 · 模板 · 礼仪）          │
│    ├─ 60 条 CR 卡点清单（分域）                │
│    ├─ PR 模板：Why / How / Testing / Risk      │
│    ├─ 评审句式：nit / consider / must-fix      │
│    └─ 单 PR ≤ 400 行 diff 上限                 │
├────────────────────────────────────────────────┤
│ Layer 1: 工具层（自动门禁）                    │
│    ├─ pre-commit：format / lint / secret-scan  │
│    ├─ CI required checks：build / test / sonar │
│    ├─ Branch protection：必须 2 人 approve     │
│    └─ Code owners：核心模块指定审查者          │
└────────────────────────────────────────────────┘

三层协同才能把 CR 从"个人自觉"升级为"组织默认"。缺任何一层，CR 都会退化：

只有 Layer 1：格式没问题但逻辑漏洞百出
只有 Layer 2：卡点清单没人执行，卡在意愿层
只有 Layer 3：没有工具/流程支撑，度量看板一直红

# 4.2 卡礼门三合一

老陈把治疗方案总结为一句话口诀：

"卡点告诉你看什么，礼仪告诉你怎么说，门禁告诉机器管什么。"

卡点（What）：60 条清单，按维度分层（正确性/可读性/安全/性能/测试）。 礼仪（How）：三种句式（nit=可选、consider=建议、must-fix=必须），前置"我"字降低攻击性。 门禁（Auto）：把机械性检查全部交给工具，人只审设计与逻辑。

关键平衡：门禁越严，人力越省，CR 越能聚焦高价值意见。

**"下集提要（本节）：**总纲清楚。下一节住院医查房——如何写一份让 reviewer 想读的好 PR。"

# 4.3 一句话回顾

卡点 + 礼仪 + 门禁三合一，是可执行、可复现、可持续的 CR 文化落地。

# 4.4 核心要点

卡点决定'必须过什么'
礼仪决定'如何说话'
门禁决定'谁把守'

# 4.5 带走清单

[ ] 制定卡点 15 条
[ ] 写下礼仪 5 条句式
[ ] 门禁自动化上线

# 5. 住院医查房

# 5.1 PR 自检十五条

目标读者：小李、小张这类日常提 PR 的初中级开发。

老陈整理了一份 "提 PR 前 15 条自检清单"，贴在 IDEA 侧边栏：

A. 尺寸与拆分（避免 F2 超大 PR）

☐ diff 是否 ≤ 400 行？（超过必拆）
☐ 每个 commit 是否有清晰主题？（feat/fix/refactor/test/chore）
☐ 是否只做"一件事"？（不要功能 + 重构 + 依赖升级混在一起）

B. 描述与上下文（避免 R06） 4. ☐ 是否填写 PR 模板 Why / How / Testing / Risk 四段？ 5. ☐ 是否关联 Jira/Issue 号？ 6. ☐ 是否有截图或示意（前端/API 变更）？

C. 自测与验证 7. ☐ 本地是否跑过全量测试并通过？ 8. ☐ 是否新增单元/集成测试？覆盖新代码 ≥ 80%？ 9. ☐ 是否手动跑过关键路径 smoke？

D. 代码基本卫生 10. ☐ 是否清理了 System.out / TODO / 注释掉的死代码？ 11. ☐ 是否用了 formatter？spotless/prettier/black 已跑？ 12. ☐ 是否检查了敏感信息？（密钥/日志中的手机号/身份证）

E. 设计与影响 13. ☐ 公共 API 是否有变更？是否兼容旧版？ 14. ☐ 数据库迁移是否可回滚？（up/down 都要） 15. ☐ 上线是否需要配置/开关/灰度？

沈总看到这份清单，第一次表扬小李："过去每次事故都是这 15 条里的某几条没做——把它当机票安全检查单。"

# 5.2 CR 意见句式

老陈教给小李、小张三种 CR 句式，帮他们从"不敢说"变"会说"：

句式 1：nit（nitpick，可选建议，非阻塞）

nit: 变量名 x 建议改成 remainingRetries，语义更清晰。 nit: 这里可以用 Optional.ofNullable() 简化两行。

句式 2：consider（建议，可讨论）

consider: 是否可以把这段循环抽成 method，命名为 filterActiveCoupons？ consider: 这里是否应该用 BigDecimal 而不是 double 处理金额？

句式 3：must-fix（必须修复，阻塞合并）

must-fix: 这里没有对 coupon.expiredAt 做校验，会导致过期券可用。 must-fix: apply() 未处理 discount > total 的场景，会返回负金额。

三种句式的礼仪原则：

对事不对人：谈代码不谈人（"这段代码" 而非 "你这段代码"）。
给方案不给情绪：指出问题时同步给出建议。
必要时点赞：好的设计明确表扬（"这个抽象很妙，学到了"）。

老陈翻出 Google Engineering Practices 里的例子：

Bad："This is terrible code." Good："nit: This method has grown quite long. Consider extracting the validation into a separate function—it'd make the main flow easier to follow."

# 5.3 初级带走清单

☐ 提 PR 前跑一遍 15 条自检
☐ PR ≤ 400 行 diff，超了就拆
☐ PR 描述填四段（Why/How/Testing/Risk）
☐ CR 意见带前缀（nit / consider / must-fix）
☐ 意见附方案，不只指问题
☐ 一次不超过 20 条意见，避免淹没重点

# 6. 主治查房

# 6.1 CR 卡点清单

目标读者：老王这类需要为团队定标准的高级开发/Tech Lead。

老陈参考 Google Engineering Practices + Karl Wiegers + 团队实际事故，编写了 60 条 CR 卡点清单，按 6 大维度分组：

# C1. 正确性（Correctness · 12 条）

编号	检查项
C1-01	是否处理了 `null` / 空集合 / 空字符串三种边界？
C1-02	循环边界是否有 off-by-one 风险？（`<` vs `<=`）
C1-03	金额/时间是否使用了 BigDecimal / Instant 而非 double / long？
C1-04	并发场景是否有共享状态？是否加锁或使用不可变结构？
C1-05	异常是否被正确处理？是否有吞异常 (`catch(Exception e){}`)？
C1-06	事务边界是否正确？是否会因 Spring 自调用失效？
C1-07	外部调用是否设置超时、重试、熔断？
C1-08	分页/大结果集是否有分页/流式处理？
C1-09	日期时区是否明确？跨时区场景是否 UTC？
C1-10	是否有资源泄漏？（连接、文件、线程）
C1-11	Idempotency：接口是否幂等或有幂等键？
C1-12	是否处理了外部输入不受信任（validation）？

# C2. 可读性（Readability · 10 条）

编号	检查项
C2-01	命名是否揭示意图？变量/函数/类都要看名知义
C2-02	函数是否 ≤ 30 行、单一职责？
C2-03	是否有魔法数字/字符串未提炼常量？
C2-04	是否有超过 3 层嵌套？可否用早返/守卫子句展平？
C2-05	注释是否在解释 Why 而非重复 What？
C2-06	是否有死代码/注释掉的旧代码？
C2-07	参数数量是否 > 4？可否封装 DTO？
C2-08	布尔参数是否已去除？（用枚举或方法拆分）
C2-09	是否有重复代码可提炼？（DRY）
C2-10	变量作用域是否最小化？（就近声明）

# C3. 安全性（Security · 10 条）

编号	检查项
C3-01	用户输入是否做参数绑定（防 SQLi）？
C3-02	输出到 HTML 是否转义（防 XSS）？
C3-03	密钥/Token 是否走环境变量而非硬编码？
C3-04	权限校验是否完整（AuthZ）？操作前置 owner check？
C3-05	敏感数据是否脱敏后再打日志？
C3-06	反序列化是否使用安全 loader？（禁用 pickle/ObjectInputStream）
C3-07	是否有 SSRF 风险？内网请求是否白名单？
C3-08	上传/下载是否限制类型与大小？
C3-09	是否有跨租户数据泄漏风险？
C3-10	依赖是否有已知 CVE？（Snyk / Dependabot）

# C4. 性能（Performance · 8 条）

编号	检查项
C4-01	是否有 N+1 查询？（JPA/Mybatis）
C4-02	循环内是否有 IO 调用？
C4-03	字符串拼接是否用 StringBuilder（大循环内）？
C4-04	索引是否覆盖新的 where 条件？
C4-05	缓存策略是否合理？失效条件是否覆盖？
C4-06	大对象是否及时释放？（避免长生命周期集合）
C4-07	是否使用了流式 / 分页 / 批量 API？
C4-08	是否有明显的算法复杂度问题？（O(n²) → O(n log n)）

# C5. 测试（Testing · 10 条）

编号	检查项
C5-01	新增代码是否有单测？分支覆盖 ≥ 70%？
C5-02	是否有变异测试或断言密度检查？
C5-03	是否覆盖边界与异常路径？
C5-04	测试是否 FIRST（Fast/Independent/Repeatable/Self-checking/Timely）？
C5-05	是否有 Testcontainers 或 in-memory 隔离？
C5-06	Mock 是否过度使用？（有无 Fake 替代）
C5-07	是否有测试命名反模式（`test1`/`testMethod`）？
C5-08	集成测试是否契约化？
C5-09	Flaky 测试是否被隔离/治理？
C5-10	覆盖率不是唯一指标，是否检查了断言强度？

# C6. 演进与运维（Ops & Evolution · 10 条）

编号	检查项
C6-01	数据库迁移是否可回滚？有 up/down 脚本？
C6-02	是否兼容旧版 API/DTO 字段？（软删除、可选字段）
C6-03	是否有 feature toggle？切换是否安全？
C6-04	日志是否结构化、有 traceId？
C6-05	Metrics/Alert 是否随代码同步更新？
C6-06	Runbook / Playbook 是否更新？
C6-07	是否有依赖升级、是否评估破坏性？
C6-08	Config 变更是否走灰度？
C6-09	是否更新了架构文档 / ADR？
C6-10	是否评估了业务影响（可观测/回滚方案）？

60 条使用建议：不是每次都过 60 条，而是根据 PR 类型抽取相关维度。例如：

前端 PR：C1 + C2 + C3（XSS/CSRF）+ C5
API PR：C1 + C3（AuthZ/输入校验）+ C4（N+1）+ C5 + C6
迁移脚本 PR：C1 + C6 全部
重构 PR：C2 + C5 + 无 API 破坏

# 6.2 评审礼仪冲突

老陈翻出 Karl Wiegers《Peer Reviews in Software》的评审礼仪三原则：

礼仪 1：分离作者与作品（Separate author from artifact）

"We're reviewing the code, not the person."

说 "这段代码可能会 NPE"，不说 "你写的代码会 NPE"
说 "考虑抽个函数"，不说 "你应该抽个函数"

礼仪 2：问问题而不是下判断（Ask, don't tell）

坏："这里应该用 Optional。"
好："这里用 Optional 是否更好？可以避免 null 判断的遗漏。"

礼仪 3：先赞后谏（Praise first, suggest second）

好的抽象、好的命名、好的测试——明确表扬
提意见前用一句 "先说好的" 缓和，例如：Nice extraction! Small nit: ...

冲突处理三步走（当作者与评审者意见分歧）：

回到原则：搬出团队之前约定的原则或 Style Guide
实验数据说话：跑 benchmark / 写反例，用数据判断
升级到第三人：请另一位资深工程师做仲裁

关键红线：任何时候都不使用人身评价、讽刺、反问语气。

# 6.3 高级带走清单

☐ 使用 60 条卡点清单，按 PR 类型抽取
☐ 意见 → 分 nit / consider / must-fix 三级
☐ 意见 → 每条附方案/示例
☐ 分离人与代码，用问句而非命令句
☐ 冲突走"原则 → 数据 → 仲裁"三步
☐ SLA 承诺 24h 内首评，不做 blocker

# 7. 主任查房

# 7.1 CR 文化建设

目标读者：沈总这类需要为整个团队/组织搭建 CR 文化的人。

沈总参考 Google/Microsoft/Facebook 的 CR 文化建设经验，提出组织级五大支柱：

支柱 1：CR 是"日常工作"而非"额外工作"

把 CR 纳入日常工作量（每人每周 CR 工作量 ≥ 20%）
PR SLA 24h 首评 → 未响应触发提醒

支柱 2：Code Owners 制度

核心模块指定 owner（GitHub CODEOWNERS 文件）
Owner 有权 block 合并、要求重设计
每季度轮换，避免 knowledge silo

# CODEOWNERS 示例
/payment/          @wangwei @laozhang
/coupon/           @xiaozhang @laowang
*.sql              @dba-team
docs/adr/          @architect-team

支柱 3：Champion & 结对 CR

每季度选 2 名"CR Champion"（意见最有价值的人）
新人入职 3 个月强制结对 CR（与 Champion 同审）

支柱 4：CR 度量看板（对齐 R08）

用 GitHub GraphQL API / Gerrit 抓取指标：

指标	目标	采集频率
首评 SLA	90% ≤ 24h	每日
单 PR 平均意见数	≥ 3 条	每日
lgtm-only 通过率	≤ 15%	每周
PR 平均 diff	≤ 400 行	每周
CR 后逃逸 Bug 率	↓ 每季度	每月

支柱 5：仪式感——评审文化的可见性

周会 5 分钟"CR 高光"：分享本周最有价值的一条评审意见
季度回顾：本季度品质事故 → 是否 CR 阶段可拦？
年度评奖：Code Health Award / Best Reviewer / Best Learner

# 7.2 门禁自动化链

沈总强调："机器能做的，不要让人做——把 60% 的检查自动化，人力聚焦剩下 40% 的高价值判断。"

Layer 1：pre-commit（本地，秒级）

# .pre-commit-config.yaml
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.5.0
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: check-yaml
      - id: check-added-large-files
        args: ['--maxkb=500']
  
  - repo: https://github.com/psf/black
    rev: 24.1.0
    hooks:
      - id: black
  
  - repo: https://github.com/pycqa/flake8
    rev: 7.0.0
    hooks:
      - id: flake8
  
  - repo: local
    hooks:
      - id: secret-scan
        name: Secret Scan
        entry: gitleaks detect --source .
        language: system

Layer 2：CI required checks（远程，分钟级）

# .github/workflows/pr-check.yml
name: PR Quality Gate

on: [pull_request]

jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-java@v4
        with: { java-version: '21' }
      
      # 1. 编译 + 测试 + 覆盖率
      - name: Build & Test
        run: ./gradlew clean build jacocoTestReport
      
      # 2. Sonar 静态扫描
      - name: SonarQube Scan
        run: ./gradlew sonar
        env:
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
      
      # 3. 依赖安全扫描
      - name: Snyk
        uses: snyk/actions/gradle@master
        with:
          args: --severity-threshold=high
      
      # 4. 变异测试（增量）
      - name: PIT Mutation
        run: ./gradlew pitest --scmroot=$(git merge-base origin/main HEAD)
      
      # 5. PR 大小检查
      - name: PR Size
        uses: CodelyTV/pr-size-labeler@v1
        with:
          xs_max_size: 100
          s_max_size: 400
          m_max_size: 800
          fail_if_xl: true

Layer 3：Branch protection（GitHub 后台）

Require pull request before merging ✅
Require 2 approvals ✅
Dismiss stale reviews when new commits push ✅
Require review from CODEOWNERS ✅
Require status checks: build / test / sonar / snyk / pit ✅
Do not allow bypass ✅

关键效果：格式/覆盖率/CVE/大 PR 全部机器拦，人只审设计与逻辑。

# 7.3 架构师带走清单

☐ 把 CR 视为日常工作量的 20%
☐ 建立 CODEOWNERS 与 24h SLA
☐ 度量看板监测 lgtm 率、首评时长、逃逸 Bug
☐ 三层门禁自动化：pre-commit / CI / branch protection
☐ 组织级仪式：Champion / 高光回顾 / 年度评奖
☐ 事故复盘每次问：CR 阶段是否可拦？

# 8. 术后康复曲线

# 8.1 通过率与耗时

Day 79-85 一周执行 CR 制度改革后，团队关键指标变化：

指标	Day 79（改革前）	Day 85（一周后）	变化
lgtm-only 通过率	87.2%	11.4%	↓ 75.8pp ✅
单 PR 平均意见数	0.6 条	4.2 条	↑ 7x ✅
PR 平均 diff	1247 行	312 行	↓ 75% ✅
平均评审耗时	6.2 分钟	28.4 分钟	↑ 4.6x ✅
首评 SLA 达成率（≤24h）	未度量	91.3%	新增 ✅
提出 must-fix 的 PR 比例	1.9%	23.7%	↑ 12x ✅

沈总看到 must-fix 率从 1.9% 升到 23.7%，第一次露出微笑：

"这个数字说明 CR 真正在起作用——每 4 个 PR 里就能拦下 1 个真问题。"

# 8.2 生产 Bug 率对照

CR 改革一周后，生产环境事故对照：

时间窗口	生产事故数	涉及模块	CR 阶段是否可拦
Day 50-79（改革前 30 天）	8 起	支付 3 / 券 2 / 库存 3	7 起可拦
Day 79-85（改革后 7 天）	1 起	前端展示 bug	属于视觉/UX 缺陷，非代码逻辑

推算 30 天期数据（保守估计）：改革后事故数 4-5 起，同比降 50%+。

老陈把这份数据写进 Day 85 复盘会：

"过去我们每季度都在'救火 → 灭火 → 复盘 → 再救火'循环里——现在 CR 把 87% 的火苗压在灶台前。"

# 8.3 一句话回顾

CR 通过率与耗时 + 生产 Bug 率对照，是 CR 文化投入产出比的唯一证据。

# 8.4 核心要点

CR 平均耗时是团队健康度信号
通过率过高说明卡点失效
生产 Bug 率反映 CR 拦截效果

# 8.5 带走清单

[ ] 每月出一次 CR 数据报表
[ ] CR 平均耗时纳入团队 KPI
[ ] 异常通过率触发文化复盘

# 9. 病案归档

# 9.1 R 系列编号

编号	名称	关键指标	治疗手段
R01	橡皮图章 lgtm	意见数 = 0	卡点清单 + 强制文字意见
R02	超大 PR	diff > 400	单 PR ≤ 400 行上限 + 拆分工具
R03	马拉松 PR	挂 > 3 天	24h SLA + PR 站会
R04	拳击场评审	意见带情绪	三句式 + 分离人和代码
R05	形式主义 CR	只挑格式	格式交 linter / 人审逻辑
R06	无 PR 描述	描述留空	PR 模板 Why/How/Testing/Risk
R07	无门禁	CI 可绕过	pre-commit + branch protection
R08	CR 无度量	指标不可见	度量看板 + 每周复盘

# 9.2 关联病案索引

A01（依赖能力）+ A02（Sonar 门禁）：CR 门禁的静态分析基础（第 7 篇）
A03（覆盖率假象）+ A04（断言密度）：CR 阶段必查测试断言强度（第 8 篇）
A05-A10（技术债类）：CR 阶段拦截未来债务的最后关口（第 9 篇）
R18（重构十八招）：CR 意见附方案时的手法参考（第 10 篇）
T01-T12（测试类）：CR 卡点清单 C5 维度直接引用（第 11 篇）

# 9.3 一句话回顾

R01-R08 编号 + 关联病案，就是团队 CR 文化的最小执行合集。

# 9.4 核心要点

每条 R 编号都可被自动化检出
编号让 CR 意见有据可查
归档是团队 CR 资产的复利

# 9.5 带走清单

[ ] R01-R08 表进 CR 模板
[ ] 团队每季度补充新编号
[ ] 把编号命中率纳入季度指标

# 10. 综合案例串讲

# 10.1 病例真相揭晓

回到本篇开头的 CouponVerifier 事件。老陈在 Day 85 复盘会上做真相回顾：

如果按新 CR 制度，这个 PR 会经历什么？

关卡 1：pre-commit（本地）

通过（格式无问题）

关卡 2：PR 提交前自检 15 条

⚠️ Fail：无单测（C5-01）
⚠️ Fail：无 PR 描述（R06）
小张会被自己的 IDEA 插件拦住

关卡 3：CI required checks

⚠️ Fail：新增代码覆盖率 0%（Jacoco 增量 gate）
⚠️ Fail：Sonar Quality Gate（增量覆盖率 < 80%）
直接 block 合并

关卡 4：Code Owner 审查（老王为 payment/coupon owner）

老王打开代码，一眼看到 apply 方法——他会给出以下意见（用新句式）：

must-fix (C1-01 边界处理):
  这里没有校验 coupon.expiredAt / status，会导致过期券可用。
  建议：if (coupon.isExpired()) throw new CouponExpiredException();

must-fix (C1-03 金额安全):
  discount 可能大于 order.total，返回负金额。
  建议：max(order.total.subtract(discount), BigDecimal.ZERO);

must-fix (C5-01 测试):
  必须补齐单测：
  - 过期券
  - discount > total
  - null coupon
  - 幂等性（同一 orderId + couponId 只扣一次）

consider (C6-05):
  建议加 metrics：coupon.applied、coupon.rejected.expired
  以便生产监控。

nit (C2-01):
  变量名 discount 可以改为 discountAmount 更明确。

Nice work on the extraction—looking forward to v2 👍

结果：合并被 block，小张补齐测试与边界，第二次 PR 合并通过。生产 47 万元损失被完全避免。

# 10.2 高质量 CR 记录

Day 84，老陈自己发了一个 PR：#3960 · CouponVerifier 二次修正（补幂等键）。以下是完整评审记录：

PR 描述：

### Why
生产事故 #INC-4711 复盘：优惠券未做幂等，触发重复扣减。

### How
- 引入 CouponUsageRecord 表（PK: orderId + couponId）
- 使用 @Transactional + insert 抛唯一键异常检测重复
- 已加 metric: coupon.duplicate.rejected

### Testing
- 新增单测 5 个（含并发场景 + Testcontainers Postgres）
- 变异测试增量分数：0.87
- 手工验证：10 线程并发下单同一 coupon，只 1 单成功

### Risk
- DB 迁移需 5 分钟停机（低峰期）
- Rollback 脚本：V2__coupon_usage_down.sql

审查记录（老王为 owner）：

15 分钟内首评（远早于 24h SLA）
共 6 条意见：1 must-fix、2 consider、3 nit
老陈修改后 3 轮往返，40 分钟合并

沈总在周会 CR 高光时段引用了这条 PR：

"**这个 PR 是我们团队从"lgtm 文化"进化到"CR 文化"的里程碑——**它告诉未来任何一位小李、小张：这里的每一行代码，都有人真正在读。"

# 10.3 设计哲学回扣

Google Engineering Practices 里有一句话：

"In doing a code review, you should make sure that: The code is well-designed. The functionality is good for the users. Any UI changes are sensible. Any parallel programming is done safely. The code isn't more complex than it needs to be. The developer isn't implementing things they might need in the future. Code has appropriate unit tests. Tests are well-designed. The developer used clear names. Comments are clear and useful. Code is appropriately documented. The code conforms to our style guides."

老陈把这段话逐条对照本篇的 60 卡点：

Well-designed → C2 可读性 + C6 演进
Functionality good → C1 正确性
Parallel safe → C1-04
Not more complex → C2-04（嵌套深度）
Not future-proof → 避免过度设计
Unit tests → C5
Clear names → C2-01
Style → 门禁自动化

核心哲学一句话：CR 不是找 Bug 大赛，是维护代码健康的日常呼吸。

Karl Wiegers 补上更深一层：

"A software peer review is not primarily to find defects—it's to improve the collective code health, share knowledge, and mentor."

CR 的三大隐性收益（Bug 拦截只是最表层）：

知识共享：每次评审都是一次跨人的知识流动
技能提升：新人通过读别人代码快速成长
文化沉淀：团队标准通过一次次评审慢慢固化

# 10.4 速查一图

四张卡片，随身可带：

卡片 1：提 PR 前自检 15 条（住院医）

尺寸：≤400 行 diff · 一事一 PR · commit 分主题
描述：Why / How / Testing / Risk 四段
自测：全量测试 pass · 新代码 ≥80% 覆盖
卫生：无死代码/println · formatter 已跑 · 无敏感信息
影响：API 兼容性 · DB 可回滚 · 灰度方案

卡片 2：60 条 CR 卡点索引（主治）

C1 正确性 (12) · C2 可读性 (10) · C3 安全 (10)
C4 性能 (8)   · C5 测试 (10) · C6 演进 (10)
按 PR 类型抽取维度，不必每次全过

卡片 3：三种 CR 句式（人文侧）

nit:        可选建议 · 非阻塞 · 示例："变量名可优化"
consider:   建议 · 可讨论 · 示例："是否抽个方法？"
must-fix:   必须修复 · 阻塞合并 · 示例："这里会 NPE"

礼仪三原则：
  1. 分离人与代码（问句而非命令句）
  2. 附方案不只指问题
  3. 先赞后谏

卡片 4：门禁三层（架构师）

Layer 1: pre-commit  → format / lint / secret-scan
Layer 2: CI checks   → build / test / sonar / snyk / pit
Layer 3: Branch prot → 2 approvals / CODEOWNERS / no bypass

度量看板：lgtm 率 / 首评 SLA / 意见数 / 逃逸 Bug 率

# 10.5 全文快速回顾

第 1 章：一次 lgtm 引发的事故，让团队直面 CR 文化的真相
第 2 章：R01-R08 八条 CR 病案 · CR 失灵的五种形态
第 3 章：CR 不用心的根因是卡点 + 礼仪双缺
第 4 章：卡点 / 礼仪 / 门禁三合一的落地方案
第 5-7 章：三视角 CR——自检十五条 / 60 条卡点 / 组织级文化建设
第 8 章：通过率、耗时、生产 Bug 率的度量对照
第 9 章：R01-R08 归档 · 与全专栏病案的关联索引

# 10.6 核心要点串联

卡点先行：无卡点则 CR 变形式
礼仪托底：无礼仪则 CR 变吵架
门禁自动化：pre-commit → CI → Branch protection
度量闭环：lgtm 率、SLA、逃逸 Bug 率必须可见
文化沉淀：CR 意见引用编号，让评审有据可依

# 10.7 常见误区汇总

❌ "lgtm" 一键放行，把 CR 当形式
❌ 只挑格式问题，逻辑与设计视而不见
❌ 评审带情绪，人与代码不分离
❌ 无 PR 描述，评审者被迫猜测意图
❌ 门禁允许绕过，CR 文化被随时突破

# 10.8 带走清单总表

[ ] R01-R08 表进入 CR 模板
[ ] PR 自检十五条随身可用
[ ] 60 条卡点按 PR 类型抽取
[ ] 门禁三层（pre-commit / CI / Branch prot）就位
[ ] 每月出一次 CR 度量看板

下集提要：

Day 86 早会，沈总在白板上写下一行字："CR 只能守住已有代码，谁来守住未来的新系统？"
团队即将启动订单聚合服务（Order Aggregator），从零构建一个新模块。沈总要求：这次不能再走 OrderMonolith 的老路——从第一行代码开始就要有免疫力。
第 13 篇《新系统免疫力》即将开始——外科第四篇 · 预防疫苗——架构守护测试 / API 契约 / 领域边界 / 演进策略，让新代码天生抗腐化。

"我们花了 12 周治好一个病人，"沈总说，"接下来 4 周，学怎么让新出生的孩子不生病。"

上次更新: 2026/07/16, 11:32:10

← 11.测试保命术全解析 13.新系统免疫力建设→