那个让我点了 Approve 的 2831 行 PR
去年 11 月，我们组的小王提了个 PR 过来，改的是订单结算那块。我到现在还记得那个数字：47 个文件变更，+2831 / −1104。GitHub 的 Files changed 页面我滚了三次才滚到底，diff 高亮把浏览器都卡了一下。
我当时的心理活动很真实：这玩意儿看下去至少两个小时，而且下周一就要发版，测试同学说功能都测过了。于是我留了一句「整体看着没问题，注意下边界」，点了那个绿色的 Approve。
两周后，财务对账对不上。查了一下午，问题出在一行代码上：
BigDecimal tax = new BigDecimal(0.13);对的，就是那个被讲烂了的坑。new BigDecimal(double) 会老老实实把 double 的二进制误差带进来，0.13 实际变成了 0.13000000000000000444089209850062616169452667236328125。乘以订单金额之后再 setScale 四舍五入，出来的分位数就飘了。正确写法是 new BigDecimal("0.13") 或者 BigDecimal.valueOf(0.13)。
这行代码在那个 PR 的第 1700 多行，夹在一堆 mapper 和 DTO 中间。我不是没能力看出来，我是根本没看到那里——人的注意力在连续滚动 1000 行 diff 之后就基本报废了，后面那一大半我全是靠「看着差不多」划过去的。（现在小王每次提 PR 都会被我念叨一遍，他大概烦死我了。）
关于「审查规模」，那些论文里的数字比你想的更狠
事后我去翻了一堆资料，有几个数字值得记一下。
一个是 Cisco 做的那个被引用烂了的研究，分析了几千次代码评审，结论是单次审查 200～400 行代码时缺陷发现率最高，能到 70%～90%；超过 400 行之后发现率断崖式下跌，到 1000 行以上基本就只剩个位数百分比了。他们还给了个审查速度上限：每小时 300～500 行，超过这个速度你只是在「看」，不是在「审」。单次时长也别超过 60 分钟，超过一小时后效率掉得很快，跟通宵写代码差不多一个道理。
另一个是微软的 Bacchelli 和 Bird 在 2013 年发的那篇（就是采访了几十个微软工程师的 MCR 研究）。我一直以为代码审查的主要产出是找 bug，结果他们发现工程师自己说的「最有价值的部分」其实是知识传递和设计讨论，找缺陷排在后面。这个结论我第一次看的时候挺不服气的，后来想明白了：如果一个 bug 需要靠人在 diff 里肉眼发现，那说明你的测试、类型系统和静态检查是有窟窿的。
还有 Google 2018 年那篇论文里的一个现象我印象很深，大意是说他们大部分变更的审查者只有 1 个人，中位延迟不到 4 小时，绝大多数改动都很小。我当时就想，人家的「小」是制度设计出来的，我们的「大」是懒出来的。（这些论文我都是二手看的，具体数字可能记岔了，感兴趣自己去搜原文，别拿我这篇当引用来源。）
我现在用的流程，一共 7 步，踩过坑的都标出来了
第一步：PR 超过 400 行直接打回，让提交者拆。 这条是我唯一强制的硬规矩。拆的原则是按「可独立回滚」来切，不是按文件类型切。比如一个功能，可以先提数据库 migration + entity，再提 service 逻辑，最后提 controller 和接口文档，每一步都能单独合并、单独回滚。刚开始同事会骂你，两个月之后他们会感谢你——因为线上出问题时，git revert 一个 200 行的 commit 比啃一个 3000 行的 PR 舒服太多。
第二步：提交者先给自己 Review 一遍。 在 GitHub 上提了 PR 之后，自己先在 Files changed 里过一遍，看到觉得「审查者可能会问」的地方，自己留个 inline comment 解释。这一步能把审查时间砍掉大概三分之一，因为很多问题是「我不知道你为什么这么写」，而不是「你写错了」。
第三步：审查顺序固定成 测试 → 接口 → 实现。 先看测试，如果测试覆盖了核心路径和边界（尤其是异常分支），那实现的审查就可以快一点，重点看命名和抽象。如果测试是空的或者只有 happy path，那这个 PR 直接打回，别浪费时间往下看。我见过太多 PR，测试里写着 assertEquals(1, 1) 这种凑覆盖率的东西，SonarQube 上一片绿，实际屁用没有。
第四步：评论分级，用 conventional comments 那套前缀。 具体就是 nit:（吹毛求疵，不改也行）、suggestion:（建议，可以讨论）、issue:（问题，必须改）、question:（我没看懂，解释一下）、praise:（这块写得好）。前面再加个 blocking: / non-blocking: 更清楚。这个习惯最大的好处是把「风格偏好」和「真问题」分开了，作者不会因为一个 nit 跟你吵半小时。
第五步：给第一轮反馈定 SLA，我们组定的是 4 小时内。 超过 4 小时没人理，提交者就会去开别的分支，然后两个分支开始互相冲突，最后合并的时候又是一场灾难。如果实在没人有空，就在群里 @ 一下，或者直接说「今天看不了，明天上午」——最怕的是 PR 挂在那里没人说话，作者不知道是没人看还是被嫌弃。
第六步：能自动化的全部自动化，别用人眼看。 我们 CI 上的门槛大概是这样：ESLint / Checkstyle 必须零 error；测试覆盖率整体门槛 60%，但增量门槛卡 70%（增量的门槛设得比整体高，防止有人吃老本）；SonarQube 的 quality gate 只卡 new code，别卡全量，不然老项目根本接不进来；cognitive complexity 超过 15 的函数会被单独报出来。至于 new BigDecimal(0.13) 这种，SpotBugs 的 DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE 规则其实就能抓到，我们当时没开而已——这个锅我背。
第七步：每两周花 30 分钟复盘「漏网之鱼」。 挑出这两周线上出过的、本来能在 Review 阶段拦住的 bug，看两件事：一是这个 bug 能不能用工具拦住，二是这类问题能不能写进团队 checklist。我们现在那份 checklist 有 23 条，最前面几条是「金额是不是用了 BigDecimal(String)」「时间是不是用了 UTC」「外部调用有没有设超时」，全是血泪换来的。
工具这事，别迷信「越贵越好」
我前后折腾过几种，直接上个对比。价格是我上次看的时候的，会变，别较真：
| 工具 | 类型 | 大概价格 | 我觉得适合谁 |
|---|---|---|---|
| GitHub 自带 Review | 托管 | 跟仓库走，私有仓库免费版也能用 | 小团队，功能其实够用，就是 diff 体验一般 |
| Gerrit | 自建 | 开源免费，但要有人维护 | 大团队、强流程、需要严格 submit rule |
| Reviewable | 托管 | 按私有仓库数收费，具体忘了 | 对 diff 可读性有执念的人，文件分组确实舒服 |
| SonarQube Community Build | 自建 | 免费，现在改成每 6 个月一个大版本 | 想吃静态分析红利又不想花钱的 |
| Danger JS | CI 里跑 | 免费 | 想把「PR 描述没写」「没加测试」这类规矩自动化 |
| CodeRabbit | AI 托管 | 印象里每人每月 20 多刀 | 单人维护的开源项目、没同事能 Review 的 |
我自己现在的组合是：GitHub Review（人）+ Danger JS（规矩）+ SonarQube（静态分析）+ ESLint/SpotBugs（语言级）。AI 那类工具我试过，它能帮你发现「这里没做空判断」「这个循环可以改成 stream」，但评论经常是噪音，尤其是那种「建议加个注释」的废话，看得人火大。如果你是独立开发、没人给你 Review，那它比没有强；如果你有同事，先把人均 Review 时间降下来更重要。
三个我不太一样的看法，可能有人不同意
第一，审查者不是越多越好，超过两个人基本是浪费。 我们组试过「关键模块必须 3 人 approve」，结果发现第三个人的评论 90% 是格式和命名偏好，实际拦截的 bug 接近于零，但 PR 的平均等待时间从 5 小时涨到了 19 小时。后来改回 2 人，出问题的概率没变化。
第二，代码审查的主要价值不是找 bug。 我前面提过那个微软的研究，但我自己的体会更直接：Review 真正救过我的，是「你为什么要在这里加一层抽象」和「这个接口以后要不要支持多币种」这类问题。真正把线上搞挂的那些 bug，八成是测试没覆盖或者需求本身理解错了，Reviewer 在 diff 里扫一眼是拦不住的。与其指望人眼，不如把测试和监控做扎实。
第三，新人入职前 30 天，我允许同步 Review。 这跟「异步优于同步」的主流说法相反。异步的好处是效率高、不用约时间，但新人不知道你们组的隐性规矩——比如为什么这个项目宁可多写 200 行也不引入某个库。前 30 天拉个 20 分钟的屏幕共享，边看边讲，比写 50 条评论快得多，也少了很多「你是不是在针对我」的误会。
几个我被问过最多的问题
「同事总在 PR 里挑格式问题怎么办？」 上 Prettier / gofmt / ktlint，配到 CI 里自动修，别让人来讨论格式。剩下的就都是 nit，作者可以不理。这不是不给面子，是团队共识。
「Review 拖了三天没人理怎么办？」 先看是不是 PR 太大，其次看是不是没写描述。一个没有任何描述的 PR，别人打开都不知道从哪看起。我们组的规矩是描述里必须写清楚「改了什么、为什么改、怎么测的」，三段，缺一段我就直接在群里要。
「LGTM 文化怎么破？」 我自己的做法是，Approved 之前至少留一条评论，哪怕只是问个问题。一条评论都不留就 approve 的，我心里默认是没看。
「AI Review 能替代人吗？」 现在不能。它擅长找模式化的错误（空指针、资源没关、明显的并发问题），不擅长判断「这个设计是不是过度了」。而后者恰恰是 Review 里最值钱的部分。
最后说一句，代码审查这事最反直觉的地方在于：你越想用它拦住所有问题，它越拦不住；你把它的边界定清楚——只拦设计、只拦关键路径，剩下的交给测试和工具——它反而变得有用。我到现在也不敢说自己会 Review，只是不那么容易点那个绿色的 Approve 了。