审查清单

这一课你将能做到

1. 拿出一份可直接复制到团队 Wiki 的 Code Review Checklist
2. 按「必须阻断合并」和「建议改进」给检查项分级
3. 把清单和 CI、PR 模板结合，让清单真正被用起来

为什么要学

人的注意力是不可靠的。同一个 reviewer，周一精神好能审出 10 个问题，周五累了只看出 2 个。Checklist 的价值是把「靠状态」变成「靠流程」——不管你是谁、什么时候审，该看的点都不会漏。

人话：飞行员起飞前再熟也要念 checklist，不是不信自己，是知道人会漏。Code Review 同理。

先建立图像

一份可落地的 CR Checklist 结构

P0 - 阻断合并（Must Fix）
├─ 功能性 bug / 逻辑错误
├─ 安全漏洞（SQL 注入、越权、密钥泄露）
├─ 数据一致性风险（缺事务、竞态）
└─ 破坏性 API 变更（无迁移方案）

P1 - 强烈建议（Should Fix）
├─ 缺失关键测试
├─ 性能问题（N+1 查询、无分页）
├─ 错误处理不当（吞异常）
└─ 文档未同步

P2 - 建议改进（Nice to Have）
├─ 命名优化
├─ 函数过长 / 嵌套过深
└─ 重复代码

P0 不解决不准合并；P1 建议解决但可协商；P2 可后续 PR 跟进。

跟着做

步骤 1：拿到完整 Checklist

## 一、功能正确性（P0）

- [ ] 代码是否实现了 PR 描述里的功能？
- [ ] 边界条件是否处理（null、空集合、0、-1、MAX_VALUE）？
- [ ] 并发场景是否有竞态（check-then-act、共享可变状态）？
- [ ] 事务边界是否正确（事务内只做 DB 操作）？
- [ ] 异常处理是否合理（无吞噬、带堆栈、有业务码）？

## 二、API 契约（P0）

- [ ] URL 是名词复数，HTTP 方法语义正确？
- [ ] 状态码语义化（201 创建、204 删除、4xx 客户端错、5xx 服务端错）？
- [ ] 向后兼容（未删字段、未改类型、未改语义）？
- [ ] 错误响应结构统一（code + message + traceId）？
- [ ] 分页参数有上限（size <= 100）？

## 三、安全（P0）

- [ ] SQL 全部参数化，无字符串拼接？
- [ ] 权限校验在每个接口入口（@PreAuthorize 或拦截器）？
- [ ] 敏感字段（密码、密钥）不出现在日志、响应、错误信息中？
- [ ] 外部输入全部校验（@Valid + 业务校验）？
- [ ] 文件上传限制类型、大小？

## 四、性能（P1）

- [ ] 无 N+1 查询（用 JOIN FETCH 或批量加载）？
- [ ] 列表接口有分页？
- [ ] 无在循环里调 DB / RPC？
- [ ] 大对象（>1MB）不放入 Redis / 内存缓存？
- [ ] 新增 SQL 有 EXPLAIN 验证索引命中？

## 五、测试（P1）

- [ ] 新增代码有对应测试？
- [ ] 测试断言针对行为（不是仅 assertNotNull）？
- [ ] 覆盖了关键边界（异常路径、并发、幂等）？
- [ ] 测试独立可重复（无顺序依赖、无共享状态）？
- [ ] CI 全部通过？

## 六、可读性（P2）

- [ ] 命名清晰（变量、函数、类）？
- [ ] 单函数 < 50 行？
- [ ] 嵌套层级 < 4 层？
- [ ] 无魔法数字？
- [ ] 注释解释 Why 而非 What？

## 七、文档（P1）

- [ ] 公开 API 有 Javadoc？
- [ ] README / API 文档与代码同步？
- [ ] CHANGELOG 或提交信息清晰？
- [ ] 数据库变更附带迁移脚本 + 回滚说明？

## 八、运维（P1）

- [ ] 关键路径有日志（入口、出口、异常）？
- [ ] 日志不含敏感信息？
- [ ] 新增配置有默认值 + 文档说明？
- [ ] 有监控/告警点（核心业务指标）？

步骤 2：把清单塞进 PR 模板

.github/pull_request_template.md：

## 改动说明
<!-- 一句话说明这个 PR 做了什么 -->

## 影响范围
<!-- 涉及哪些模块/接口/数据表 -->

## 自测清单
作者提交前必须自查：
- [ ] 本地跑过 `mvn test` 全部通过
- [ ] 本地跑过 `mvn spotless:check` 无风格问题
- [ ] 新增接口已在 Postman 验证
- [ ] 数据库变更已在本地从空库验证过

## Reviewer 清单
Reviewer 请对照团队 Checklist 逐项检查：
- [ ] 功能正确性（P0）
- [ ] API 契约（P0）
- [ ] 安全（P0）
- [ ] 性能（P1）
- [ ] 测试（P1）
- [ ] 文档（P1）

## 注意事项
<!-- 需要 reviewer 特别关注的点 -->

效果：作者提 PR 时必须面对这份清单；reviewer 审时有明确指引。

步骤 3：用 CI 把能自动化的项自动化

清单里很多项不应该靠人查：

检查项	自动化方式
SQL 注入	SonarQube 静态扫描
密钥泄露	gitleaks / truffleHog 扫 diff
测试覆盖率	JaCoCo + CI 卡阈值（如 >= 70%）
代码风格	Checkstyle / Spotless
依赖漏洞	OWASP Dependency-Check
API 契约变更	springdoc-openapi 导出对比

CI 配置（GitHub Actions 片段）：

- name: Run tests with coverage
  run: mvn verify jacoco:report

- name: Check coverage threshold
  run: |
    COVERAGE=$(grep -oPm1 '(?<=<counter type="LINE" missed=")[0-9]+' target/site/jacoco/jacoco.xml)
    # 计算并判断是否达标，不达标 exit 1

- name: Secret scan
  uses: gitleaks/gitleaks-action@v2

- name: SonarQube analysis
  run: mvn sonar:sonar -Dsonar.projectKey=myproject

原则：CI 能查的，人就不查。人只查 CI 查不了的（设计、语义、业务逻辑）。

步骤 4：定期回顾和修剪清单

每季度回顾一次：

□ 哪些项从来没被触发过？（可能该删）
□ 哪些项经常被触发？（考虑提到更高优先级，或加 CI 自动化）
□ 最近线上事故有没有对应检查项？（没有就补）
□ 新人能不能照着清单完成 review？（不能就改写法）

清单不是越多越好——超过 30 项就没人看了。精简到 15-20 项最关键的，比 50 项大而全的有效得多。

讲清楚要点

1. 清单要分级：P0 阻断合并、P1 强烈建议、P2 可选。不分级的清单等于没分级——reviewer 会把所有项当成可选项。
2. 能自动化的不要人工：CI 每多拦一项，reviewer 就少一项负担。
3. 清单要嵌入流程：写进 PR 模板、嵌入 CI、作为 review 评论模板。放在 Wiki 角落的清单等于没有。
4. 清单是活的：每个季度根据实际事故和团队反馈修剪。
5. 作者先自查，reviewer 再复查：PR 模板里加作者自查清单，能过滤掉 50% 的低级问题。

你可能会懵的地方

• 清单会不会让 reviewer 变成「勾选项机器」？ 会，如果清单太长或太琐碎。解法：只保留真正重要的项，且鼓励 reviewer 在清单之外思考（「这个设计合理吗」这种问题清单管不了）。清单是底线，不是上限。

• 小团队需要这么正式的清单吗？ 需要，但可以精简。3 人团队可能只需要 10 项核心清单。重点是「写下来」，让团队对「什么叫审过了」有共识。

• ** reviewer 没按清单审，出了问题算谁的？** 不要把清单变成追责工具。清单的目的是「减少漏审」，不是「出事后分锅」。如果清单变追责工具，reviewer 会变得保守——什么都不敢 approve，效率反而下降。

• 怎么判断清单项该放 P0 还是 P1？ 问一个问题：「这个问题合并后会造成什么后果？」线上事故、数据丢失、安全事故 → P0；可维护性下降、性能隐患 → P1；纯美观 → P2。

• 清单和 reviewer 的经验哪个重要？ 互补。清单保证底线（不漏低级问题），经验决定上限（发现设计层面的问题）。新人靠清单快速成长，老人用经验突破清单。

错法对照

错法	问题	正解
50 项大而全清单	没人看	精简到 15-20 项核心
清单不分级	所有项都是可选	P0/P1/P2 三级
清单只放 Wiki	没人记得看	嵌入 PR 模板
全靠人工检查	漏检率高	CI 自动化兜底
清单多年不更新	与实际脱节	每季度回顾修剪

卡点急救

1. Reviewer 抱怨清单太长：砍。只留 P0 全量 + P1 高频项。剩下的做成「详细版清单」链接放在 PR 模板里供参考。

2. 清单项经常被忽略：检查是不是没嵌入流程。如果清单只在 Wiki 里，等于没有。必须在 PR 模板里、CI 里、review 评论里都能看见。

3. 新人不知道清单项怎么判断：给每个清单项配一个「反例 → 正例」的代码片段。看一遍例子比读十遍描述有效。

面试常问

• 你们的 Code Review Checklist 有哪些项？ 按 P0/P1/P2 分层讲。P0 讲「功能正确性 + API 契约 + 安全」三大块，每块举 2-3 个具体例子（比如「并发是否有竞态」「SQL 是否参数化」）。展现你理解「为什么这些项是 P0」。

• 怎么让团队真正用起来 Checklist？ 三步：① 嵌入 PR 模板让作者自查；② CI 自动化能自动化的项；③ 定期回顾修剪，根据线上事故补项。关键是「嵌入流程」而不是「倡导自觉」。

• Checklist 会不会限制 reviewer 的思考？ 会，如果清单被当成「全部」。好的用法是「清单是底线，reviewer 在清单之外还要思考设计、语义、长期影响」。同时清单要定期更新，把 reviewer 反复发现的新问题固化进去。

自测三问

1. P0 和 P1 检查项的区别是什么？各举两个例子。
2. 为什么说「CI 能查的，人就不查」？
3. 清单多久应该回顾一次？回顾时看哪几个维度？

小结一句话

审查清单的价值是把「靠状态」变成「靠流程」——分级、嵌入、自动化、定期修剪，让每个 PR 都被同一把尺子量过。

延伸

• Google Engineering Practices - Reviewer Guide
• OWASP Code Review Guide
• 下一课：安全审查——OWASP 与密钥泄露。