大家好,我是程序员柏风,AI 最全的 Vibecoding 学习地图 http://coderoadmap.cn 作者。
综合练习
这一课你将能做到
- 拿到一个真实 PR,独立完成从「快速扫读」到「深度审查」的完整流程
- 输出一份结构化的 review 报告,包含问题清单、优先级、修改建议
- 识别本站在 Spring Boot + MySQL + Redis + Next.js 技术栈下的典型问题模式
为什么要学
前面 11 课分别讲了 API、实现、文档、测试、风格、流程、清单、安全、AI、反馈、团队。这一课把所有知识点串起来,用一个完整的实战案例检验你是否真的会审。
人话:学了一堆理论,不如亲手审一个 PR。这一课就是你的「毕业考试」。
先建立图像
一个完整 PR 的审查动线
第一步:快速扫读(5 分钟)
├─ 看 PR 描述:做了什么、为什么、影响范围
├─ 看文件列表:改了哪些文件、多大
└─ 看 CI 状态:测试过了吗、Lint 过了吗
第二步:定位高风险区域(10 分钟)
├─ 新增/修改接口 → API 语义审查
├─ 涉及数据库 → SQL、事务、索引
├─ 涉及缓存 → 一致性、穿透/击穿/雪崩
├─ 涉及外部调用 → 超时、重试、降级
└─ 涉及权限 → 鉴权、越权
第三步:深度审查(20 分钟)
├─ 逐行读代码,对照 Checklist
├─ 思考「如果输入是 X 会怎样」
└─ 思考「如果并发/失败/超时了会怎样」
第四步:输出报告(5 分钟)
├─ P0 阻断项
├─ P1 强烈建议
└─ P2 建议改进
跟着做
练习:审查本站「课程发布」PR
假设你收到这样一个 PR:
标题:feat(course): 新增课程发布功能
描述:讲师可以发布课程,发布后 C 端可见,同时更新搜索索引
改动文件:
- CourseController.java(+50 行)
- CourseService.java(+120 行)
- CourseMapper.java(+20 行)
- CourseMapper.xml(+30 行)
- CourseServiceTest.java(+80 行)
- application.yml(+5 行)
第一步:快速扫读
✓ PR 大小:305 行,在合理范围内
✓ CI 状态:全部通过
✗ PR 描述:太简略,没说「发布失败怎么处理」「索引更新失败怎么办」
动作:先评论一句
[question] PR 描述里没提到「发布失败时状态是否回滚」
和「搜索索引更新失败的处理策略」,能否补充一下?
这会影响我对异常处理逻辑的审查重点。
第二步:定位高风险区域
高风险点:
1. CourseService.publish() → 事务边界、状态变更
2. searchClient.index() → 外部调用、失败处理
3. CourseMapper.xml 新 SQL → 索引、性能
4. 权限校验 → 只有课程作者能发布吗?
第三步:深度审查
打开 CourseService.java,看到:
@Transactional
public void publishCourse(Long courseId, Long userId) {
Course course = courseMapper.selectById(courseId);
if (course == null) {
throw new BizException("课程不存在");
}
if (!course.getAuthorId().equals(userId)) {
throw new BizException("只有作者可以发布");
}
if (course.getStatus() == CourseStatus.PUBLISHED) {
throw new BizException("课程已发布");
}
courseMapper.updateStatus(courseId, CourseStatus.PUBLISHED);
searchClient.index(courseId); // ← 注意这里
mqProducer.send(new CoursePublishedEvent(courseId)); // ← 还有这里
}
问题识别:
[P0] 事务内调用外部服务
- searchClient.index() 是 ES 调用,可能超时
- mqProducer.send() 是 MQ 调用,可能失败
- 两者都在 @Transactional 内,会导致事务超长或消息先于提交发出
[P0] 权限校验在 Service 层,但 Controller 层没有 @PreAuthorize
- 如果其他 Service 调用这个方法,权限校验会被绕过
- 建议:Controller 层加 @PreAuthorize,Service 层保留业务校验
[P1] 课程状态变更没有乐观锁
- 两个请求同时发布,可能导致状态不一致
- 建议:updateStatus 加 WHERE status = DRAFT 条件
[P1] searchClient.index() 失败会怎样?
- 如果 ES 挂了,整个发布会失败,但 DB 已提交
- 建议:用 Outbox 模式,或 try-catch + 异步重试
[P2] 异常信息可以更明确
- "课程不存在" 可以带上 courseId
- "只有作者可以发布" 可以带上当前用户和作者 ID
打开 CourseMapper.xml:
<update id="updateStatus">
UPDATE course
SET status = #{status}, update_time = NOW()
WHERE id = #{courseId}
</update>
问题识别:
[P1] 缺少 WHERE 条件防止并发更新
- 两个请求同时调用,都会成功
- 建议:WHERE id = #{courseId} AND status = 'DRAFT'
[P1] 没有检查 course 表的 status 字段索引
- 如果后续有「查询所有已发布课程」的需求,需要索引
- 建议:确认是否有 idx_status 索引,或本次加上
打开 CourseServiceTest.java:
@Test
void publishCourse_success() {
courseService.publishCourse(1L, 100L);
Course course = courseMapper.selectById(1L);
assertEquals(CourseStatus.PUBLISHED, course.getStatus());
}
问题识别:
[P0] 测试只覆盖了 happy path
- 缺少:课程不存在、无权限、重复发布、并发发布
- 缺少:searchClient 失败、mqProducer 失败
- 建议:补充至少 5 个异常场景测试
[P1] 测试没有验证外部调用
- searchClient.index() 是否被调用?
- mqProducer.send() 是否被调用?
- 建议:用 Mockito verify 验证
第四步:输出结构化 Review 报告
## Review 总结
**整体评价**:功能实现正确,但存在事务边界和并发安全问题,需要修改后才能合并。
**优点**:
- 权限校验逻辑清晰
- 代码结构符合分层规范
- 有基本的单元测试
**必须修改(P0)**:
1. **事务内调用外部服务**
- 位置:CourseService.publish() 第 18-19 行
- 问题:searchClient.index() 和 mqProducer.send() 在 @Transactional 内
- 建议:用 Outbox 模式,或事务提交后异步执行
- 参考:https://microservices.io/patterns/data/transactional-outbox.html
2. **测试覆盖不足**
- 位置:CourseServiceTest.java
- 问题:只覆盖 happy path,缺少异常场景
- 建议:补充课程不存在、无权限、重复发布、并发发布、外部服务失败等场景
**强烈建议(P1)**:
3. **并发更新缺少保护**
- 位置:CourseMapper.xml updateStatus
- 建议:WHERE 条件加 `AND status = 'DRAFT'`
4. **权限校验层级**
- 位置:CourseController.publish()
- 建议:Controller 层加 @PreAuthorize,Service 层保留业务校验
5. **外部调用失败处理**
- 位置:CourseService.publish() searchClient.index()
- 建议:明确失败策略(回滚 or 重试 or 降级)
**建议改进(P2)**:
6. 异常信息带上关键参数(courseId、userId)
7. 考虑给 course.status 字段加索引
**下一步**:
请作者优先处理 P0 问题,修改后我会复审。P1 问题建议本次解决,
P2 问题可以后续 PR 跟进。
讲清楚要点
- 先快速扫读,再深度审查:不要一上来就逐行读代码。先花 5 分钟理解「这个 PR 要做什么」,再定位高风险区域。
- 高风险区域优先:接口变更、数据库操作、外部调用、权限校验——这四个区域出问题代价最大,优先审查。
- 用 Checklist 兜底:审完高风险区域后,对照 Checklist 快速过一遍其他区域,确保不漏。
- 输出结构化报告:P0/P1/P2 分级、每个问题带位置/原因/建议、明确「必须修改」和「可以后续」的边界。
- 正面反馈不能少:指出问题的同时,也要认可写得好的部分(「权限校验逻辑清晰」「代码结构规范」)。
你可能会懵的地方
-
Review 一个 PR 应该花多长时间? 经验值:PR 行数 ÷ 20 = 分钟数。300 行的 PR 大约 15 分钟。超过 400 行的 PR 建议要求作者拆分,而不是硬着头皮审。
-
发现问题但不确定是不是问题怎么办? 标记为
[question]或[uncertain]提出来:「我不确定这里在并发场景下是否有问题,你验证过吗?」让作者去确认,比自己猜半天高效。 -
作者不认同我的审查意见怎么办? 参考「反馈技巧」课:先确认是不是理解层面的问题(换种方式解释),再确认是不是观点层面的分歧(尊重作者决定),最后才是事实层面的分歧(写 benchmark 或 POC 验证)。
-
Review 时发现代码风格有问题,但 Lint 没报,怎么办? 两种处理:① 如果是团队规范里有的,指出来让作者改;② 如果是团队规范里没有的,提个 PR 更新 Lint 配置,而不是在业务 PR 里争论。
-
Review 通过后线上出了问题, reviewer 有责任吗? 有责任,但不是全部责任。Review 是「降低风险」而不是「消除风险」。线上问题的根本原因是「测试覆盖不足 + 监控告警缺失 + 回滚机制不完善」。Review 只是其中一环。
错法对照
| 错法 | 问题 | 正解 |
|---|---|---|
| 一上来就逐行读代码 | 效率低,抓不住重点 | 先扫读 → 定位高风险 → 深度审查 |
| 只看实现不看测试 | 漏掉测试覆盖不足 | 代码和测试一起审 |
| 所有问题都标 P0 | 作者不知道优先级 | P0/P1/P2 分级 |
| 只挑刺不表扬 | 作者抵触 | 认可好的部分 |
| Review 通过就不管了 | 线上出问题才追悔 | 关注合并后的监控和回滚 |
卡点急救
-
PR 太大审不动:直接评论「建议拆分为多个 PR,每个 PR 不超过 400 行。可以先拆出 XX 模块作为第一个 PR」。不要硬着头皮审,敷衍的 review 比不审更糟糕。
-
看不懂作者的实现思路:直接问:「能否在 PR 描述里补充一下设计思路?特别是 XX 部分为什么用这种方式实现?」不要猜,猜错了浪费时间。
-
Review 完发现问题很多,怕打击作者:分级表达。P0 问题直接说「必须修改」;P1 问题说「强烈建议」;P2 问题说「可选」。同时认可写得好的部分,让作者知道「不是全盘否定」。
面试常问
-
描述一次你 Review 时发现重大问题的经历 用 STAR 法则:Situation(什么项目、什么功能)、Task(你负责 review 这个 PR)、Action(你怎么审的、发现了什么问题、怎么沟通的)、Result(问题修复了、避免了什么损失、团队学到了什么)。重点是「怎么发现的」和「怎么沟通的」,而不是「我多厉害」。
-
Review 时如何平衡「质量」和「速度」? 参考「团队实践」课:控制 PR 大小(小 PR 审得快且质量高)、CI 前置(不过 CI 不审)、分层审查(低风险 PR 中级工程师审)、时间盒(资深每天固定时间 review)。核心观点:速度和质量不矛盾,PR 越小两者都越好。
-
如果让你设计一个团队的 Code Review 流程,你会怎么做? 分四层:① 工具层(CI + Lint + Checklist + CODEOWNERS);② 流程层(PR 大小限制 + SLA + 紧急通道);③ 文化层(新人培养 + 正面反馈 + 定期回顾);④ 监控层(Review 健康度仪表盘 + 周报)。强调「让正确的行为成为默认行为」。
自测三问
- 审查一个 PR 的完整流程是什么?(四步)
- 哪些区域是「高风险区域」,需要优先审查?
- 输出 Review 报告时,P0/P1/P2 分别代表什么?
小结一句话
综合练习的核心是「把 11 课的知识串成一条动线」——快速扫读定位重点、深度审查高风险区域、Checklist 兜底、结构化报告输出。
延伸
- Google Engineering Practices - Code Review Developer Guide
- 《Best Kept Secrets of Peer Code Review》(Jason Cohen)
- 本站 Code Review 路线图——回到路线图,标记你已掌握的节点
毕业致辞
恭喜完成 code-review-essentials 全部 12 课。你现在应该能够:
- 独立审查一个 Spring Boot + MySQL + Redis + Next.js 项目的 PR
- 识别 API、实现、安全、测试四个维度的高风险问题
- 用结构化的方式输出 review 报告
- 在团队内推动 review 流程和文化的建设
下一步建议:
- 在实际项目中找 3 个 PR 练习审查,应用本课程的 Checklist
- 和团队分享你的 review 报告,收集反馈
- 每月回顾一次「我审过的 PR 后来有没有出问题」,持续校准你的审查标准
Code Review 是一项「越做越好」的技能。保持怀疑、保持好奇、保持善意。