审查清单
这一课你将能做到
- 拿出一份可直接复制到团队 Wiki 的 Code Review Checklist
- 按「必须阻断合并」和「建议改进」给检查项分级
- 把清单和 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 项大而全的有效得多。
讲清楚要点
- 清单要分级：P0 阻断合并、P1 强烈建议、P2 可选。不分级的清单等于没分级——reviewer 会把所有项当成可选项。
- 能自动化的不要人工：CI 每多拦一项，reviewer 就少一项负担。
- 清单要嵌入流程：写进 PR 模板、嵌入 CI、作为 review 评论模板。放在 Wiki 角落的清单等于没有。
- 清单是活的：每个季度根据实际事故和团队反馈修剪。
- 作者先自查，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 自动化兜底 |
| 清单多年不更新 | 与实际脱节 | 每季度回顾修剪 |
卡点急救
-
Reviewer 抱怨清单太长：砍。只留 P0 全量 + P1 高频项。剩下的做成「详细版清单」链接放在 PR 模板里供参考。
-
清单项经常被忽略：检查是不是没嵌入流程。如果清单只在 Wiki 里，等于没有。必须在 PR 模板里、CI 里、review 评论里都能看见。
-
新人不知道清单项怎么判断：给每个清单项配一个「反例 → 正例」的代码片段。看一遍例子比读十遍描述有效。
面试常问
-
你们的 Code Review Checklist 有哪些项？
按 P0/P1/P2 分层讲。P0 讲「功能正确性 + API 契约 + 安全」三大块，每块举 2-3 个具体例子（比如「并发是否有竞态」「SQL 是否参数化」）。展现你理解「为什么这些项是 P0」。
-
怎么让团队真正用起来 Checklist？
三步：① 嵌入 PR 模板让作者自查；② CI 自动化能自动化的项；③ 定期回顾修剪，根据线上事故补项。关键是「嵌入流程」而不是「倡导自觉」。
-
Checklist 会不会限制 reviewer 的思考？
会，如果清单被当成「全部」。好的用法是「清单是底线，reviewer 在清单之外还要思考设计、语义、长期影响」。同时清单要定期更新，把 reviewer 反复发现的新问题固化进去。
自测三问
- P0 和 P1 检查项的区别是什么？各举两个例子。
- 为什么说「CI 能查的，人就不查」？
- 清单多久应该回顾一次？回顾时看哪几个维度？
小结一句话
审查清单的价值是把「靠状态」变成「靠流程」——分级、嵌入、自动化、定期修剪，让每个 PR 都被同一把尺子量过。
延伸