1. 先说说我对代码评审这件事的重新定义
1.1 代码评审不是找茬，是一场效率博弈
我进入这个行业已经有十多年，代码评审这东西从最开始流行的"结对编程变体"，到后来各家团队摸索出的各种流程规范，我基本都经历过。如果让我用一句话总结这些年的感受，那就是：能把代码评审做好、做持久、做出价值的团队，一定是把它当效率工具在经营，而不是当QA流程在应付。
为什么这么说？我见过太多团队把Review当成"上线前的最后一道关卡"，评审会开得像批斗会，评论区长满了"这里为什么要这么写""建议改成xxx"这类没有上下文的短句，reviewer自己都没看明白调用链就凭感觉说话。这种评审，不仅没有提高代码质量，反而把写代码的人搞得畏手畏脚，把审代码的人搞得意兴阑珊。最终结果就是：Review从"质量保障手段"退化成了"流程形式主义"。
反过来看，我待过的一个比较成熟的团队，他们对Review的理解非常直白：你写的代码，是给下一个维护它的人看的，评审就是提前替那个人把疑问问出来。想通了这一点，很多流程设计就都顺了。
1.2 我见过最典型的三种错误心态
第一种，把Review当成"审核关卡"。评审人默认代码是有罪的，作者需要自证清白。这种心态下，reviewer会钻牛角尖，作者会防御性解释，双方最后都在争"面子"而不是"代码质量"。
第二种，把Review当成"教学现场"。资深工程师看到新人的代码，忍不住逐行指点，一次Review写下几十条评论。出发点是好的，但效果往往很差——新人被淹没在意见海里，反而抓不住重点。我一直认为，一次评审能带出3到5条真正重要的意见，就已经很成功了。剩下的细节问题，完全可以通过静态检查、结对编程、或者另一个小迭代去消化。
第三种，把Review当成"走过场"。所有人都知道在看，但所有人都没认真看，看到绿勾就点通过。这种情况最危险，因为它会让整个团队对评审机制失去信任，一旦有了"反正没人仔细看"的共识，垃圾代码就会流向主干，回头再来补债，成本高好几倍。
我不打算再展开说这些心态有多糟糕，因为我写这篇博文的主要原因，是想分享这半年多来我在推进一个内部代码评审改进计划时，沉淀下来的具体做法和踩过的坑。从定义评审目标、设计流程、量化指标，到处理评审中的情绪问题和工具链替代，每一块我都试过不少方案。这些东西不是论文里的框架，是我和团队一步步调出来的，写出来供同行参考。
2. 一套能让评审不那么痛苦的提交与分配机制
2.1 提交之前：把改动描述当产品文档写
我在Review实践里踩过最痛的一个坑：有一次某工程师提交了一个改动，改动说明只写了两个字"update"，连标题都是系统自动生成的分支名。我点开代码一看，改动涉及一个核心服务的配置结构、一个数据迁移脚本、还顺带重构了一个工具函数。我花了将近四十分钟才拼凑出他到底想干什么，而且中途还判断错了一个方向，给了完全错误意见。那次之后我定了一条规则：改动说明写不清楚的PR，reviewer有权利直接打回，不用替作者猜。
具体来说，我觉得一个合格的PR描述至少要包含这几块内容：
- 背景：为什么要做这个改动，是响应了哪个需求、修复了哪个线上问题，还是单纯的技术债清理；
- 改动概览：涉及了哪些模块，每个模块大致做了什么；
- 风险点：有没有影响兼容性、有没有改到公共方法、有没有涉及数据迁移；
- 测试情况：本地跑了哪些用例、有没有手动验证过的场景。
这里我想多说一句"测试情况"。很多团队Review时最焦虑的就是"我看了半天也没法确定这个改动到底跑没跑过"。如果作者在描述里能明确写出"我在xx场景下用xx数据验证过，结果符合预期"，reviewer的状态会完全不一样——他从"怀疑一切"变成"针对性地看某几个地方"，效率提升非常明显。
我在内部推动这事的时候，没有直接要求大家写长篇大论，而是定了一个模板，字段不多不少就四行，默认值都是"无"，逼着提交的人强制过一遍脑子。实行两周后，Review评论里"这是什么改动""为什么改这里"这类问题占比下降了大概六成。
2.2 改动越小，评审越快
这几乎是所有评审效率问题的核心，但恰恰最难执行。很多人嘴上认同"小步提交"，实际操作中却总是攒一个大分支，等所有功能都做完再开PR。原因也简单——拆分是反人性的，人总是倾向于在脑子里把一个功能想完整了再动手，代码也一样，写到一个"自认为可以见人"的状态才愿意拿出来。
解决这个问题，我必须诚实地讲：没有魔法，只有硬性约束。我们团队后来设了一个红线——单个PR的改动量，新增加删除的行数，一般不超过400行，涉及架构调整的大改动例外，但例外必须提前在群里说明。超了的主管会找你聊原因，不算惩罚，就是帮你去拆。这个数字不是拍脑袋定的，参考了行业里一些公开的经验数据——改动量越大，reviewer每行代码投入的注意力就越少，术语叫"评审密度"下降。一个小改动，评审人能看到80%的细节；一个几千行的大改动，能看个大概就不错了。
拆分还有一个好处：出问题容易回滚和定位。有一次我们把一次重构拆成了四个连续的PR，前三个合进去之后线上监控数据平稳，最后一个出了问题，直接回滚最后一个，前三个保留。如果在同一个大PR里做，回滚就是牵一发动全身。
2.3 谁来Review、何时Review，大部分团队都没定规则
很多团队的做法是"谁有空谁看"，或者在群里吼一嗓子"帮忙review一下"。这样做的直接后果是：热门模块的代码反复被同两三个人看，冷门模块的代码常年没人认真检查。
我后来参考了一个开源社区推荐模型，给团队定了一套简单的规则：
- 每个PR至少两个reviewer，其中一个必须是该模块的"熟悉者"（怎么定义？过去一个月在这个目录下有最多提交记录的人），另一个可以是任意工程师，用来补充外部视角；
- Review响应时间不超过四个工作时，超过就算延误，自动化工具会把PR顶到群置顶；
- 作者在非工作时间发的PR，不强制当天响应，避免大家下班了还盯着手机审代码。
这里我想展开说一下"外部视角"。为什么需要一个不了解这块代码的人来review？因为我发现，模块熟悉者很容易陷入"惯性理解"——他看到某段代码会觉得"哦，这里本来就是这么写的"，默认一切合理。而一个没写过这块代码的人，会真的傻乎乎地去追每一个函数调用，结果反而最容易发现"这个函数原来不是这个语义""这里有个边界条件没处理"这类问题。这和我们平时说的"代码评审要看新人视角"是一个道理。
至于响应时长的约束，说实话一开始阻力不小，有人觉得"我在专注写功能，不想被打断"。但后来我们把期待调整为"四小时内给出初步回应，哪怕结论是你现在很忙、最晚明早看"，大家就都能接受了。关键是不能让人发完PR就一直悬在那里，得不到任何反馈。
3. 评审过程里最容易被忽略的技术判断点
3.1 从"这段代码对不对"到"这段代码该不该存在"
评审技巧这东西，很多技术文章都在讲"你要看并发安全问题""你要看边界条件"，这些当然都对。但我想说一个很少被系统化强调的点：最高级的评审意见，是能指出这段代码压根不该写。
什么意思？我举一个真实的例子。某团队有个模块，每次调用外部服务都要先拼一个很长的签名串，代码里重复了五六遍。A同学某次提交，把这套签名逻辑抽成了一个公共工具类，然后所有调用处都改成了调用新方法。从代码正确性角度讲，这个改动没有毛病，测试也过了，压缩重复代码本身也是好事。但review时一个资深的工程师提了个问题：为什么我们不把这个签名计算放到基础库里，让所有服务统一使用，而是只在这个服务内部抽个工具类？这一问，变成了一个架构评审议题——这个工具类，最后真的下沉到了公共模块，收益翻了不止一倍。
所以我在评审时给自己定了两个原则：
- 每看一段新增代码，先问一句"这个功能本来是不是已经有现成的东西能实现了？"很多重复的轮子，不是不知道去哪里找，而是没人愿意花十分钟翻一下现有的依赖；
- 每看到一次删除代码，会特别注意——如果删掉的是公共接口，会不会有其他模块在调用？这里需要多跑一步全局搜索，别只看着当前PR的代码。
这个维度写不进自动化检查工具，恰恰是Review作为"人肉活动"的核心价值。
3.2 并发、幂等、失败路径：三类高发问题
在看代码评审意见时我统计过一个大概印象——除了纯逻辑错误，引发线上事故最多的问题集中在三类：并发处理、幂等性、失败路径。
并发问题的典型形态是：新来的工程师写了个带状态的对象，在多线程环境里直接用，没有加锁也没有做线程封闭。这类问题看着代码很难发现，因为本地测试不会触发。我的建议是：review时凡是看到静态变量、共享的单例、或者写入全局容器的地方，都下意识问一句"这个会不会被多个线程同时碰？"
幂等性问题在接口改造中尤其高发。比如支付回调这个场景，回调可能会因为网络原因来两次、甚至三次。如果处理逻辑没有做幂等，就可能在重复退款、重复扣费这种级别的事故里栽跟头。Review时我会特意看：这个接口是不是天然可能被重复调用？如果是，它的去重逻辑在哪？
失败路径则是另一个容易忽视的点。很多代码写的是"happy path"，主线一路顺畅能跑通，但网络超时、缓存击穿、下游返回异常数据这些情况，往往没有覆盖。我看过一个印象很深的PR——做了缓存管理的重构，但完全没考虑Redis临时不可用怎么办，代码直接抛异常。review意见只需要简单提一句"降级策略是什么"，作者可能就会反应过来，补上一个兜底逻辑。
这三类问题有一个共同点：都是无法通过"看代码对不对"来发现的，必须靠reviewer主动拉高视角，想象这段代码在真实运行环境里的样子。所以我一直觉得，好的reviewer本质上是在脑子里做一次"演算"，而不是在逐行做"检查"。
3.3 如何提意见，别人才愿意改
这里我想说一些比较实际的东西。同样的意见，不同的表述方式，效果差很远。
我观察到一个现象：很多工程师提意见时喜欢用"命令式短句"，比如"这里改成用缓存""去掉这个魔法数""用Optional"。这些话技术上是没毛病的，但接收方容易产生一种被指挥的感觉，尤其是在双方资历有差距的时候。
我后来练习了一种表述结构，简单说就是"原因-建议-开放式结尾"：
- 先说明我为什么这么提，比如"这个函数在循环里被调用了上千次，每次创建新对象开销有点大"；
- 再给出具体建议，"可以考虑提出来缓存一份"；
- 最后补一句开放式的，"你觉得呢？或者你了解过这里的数据量吗？"
这样做的效果很神奇。接收方不会觉得被命令，反而会带着"要不要接受这个建议"的心态去看，讨论质量明显提升。
另外还有一个很实用的小技巧——评论尽量绑定到具体的代码行上，不要只写一个总结性的长篇大论放在PR最后。绑定到代码行的评论，作者改动的时候方便对照，讨论时上下文也清晰。放在最后的大段总结，作者大概率会看，但很难形成一对一的回应。
如果有人问我"Review意见收到后不知道怎么回复怎么办"，我会建议最简单的做法：每条意见回一个"改"或者"不改，原因是xxx"。不需要长篇大论的辩护，也不需要卑微的道歉。Review讨论的本质是信息交换，不是辩论赛，更不是法庭审判。
4. 那些让评审形同虚设的坑和真实排查
4.1 "已通过"了，但谁都没认真看的假评审
有一段时间我发现一个比较隐蔽的问题：某小组的代码质量没有变好，但Review通过率一直是100%，评论数低得异常。我一开始以为他们代码写得好，后来偶然翻了一下评论记录，发现大量PR的reviewer点开没几分钟就通过了，有些评论内容甚至是空模板。
这种情况就是典型的"假评审"。它比"不评审"更可怕，因为它制造了安全假象。
排查原因的时候，我发现两个核心问题：
- reviewer人选和作者是同一个"小圈子"里互相吹捧的伙伴，不愿意给对方制造麻烦；
- reviewer自己心里没底，尤其是刚加入团队、对系统还不熟悉的人，他觉得"我看了半天没看出问题，别是我水平不够，那就算了通过吧"。
针对第一种，我们调整了分配规则，引入了"跨组Review"——不是本小组的人来当第二评审人。针对第二种，我们在新人入职的前两周不强制安排独立Review，跟着老人看、写意见草稿，由老人过目后再发。同时把"发表意见"的心理门槛降低了，明确告诉所有人："不确定是不是问题的意见，也欢迎提出来，用疑问句就行，作者自己会判断。"
4.2 Review疲劳：意见说多了自己也累
Review这事，其实挺吃心力的。一次认真投入、有信息量的Review，可能需要半小时甚至更久，连续做几个之后，大脑注意力就会明显下降。我们团队有过一个阶段，几个人连续两周高强度评审，结果后几天开始陆续出现"只挑语法毛病"的评论，比如缩进不对、变量命名不好，而真正的逻辑漏洞反而没人提。
这是认知疲劳在作祟。人的精力有限，前几个PR消耗掉大部分精力，后面就进入"省电模式"。
解决Review疲劳，我从两个方向入手：
- 控制单日评审量——设置了每天评审的PR数量上限，超过上限的自动转到第二天，或者分流给其他有精力的工程师。这里需要团队有一定的人力冗余才能扛得住，但哪怕只是明确这个意识，也比硬撑要好；
- 把简单的流程动作机器化——格式化检查、静态规则扫描、测试覆盖率门禁，这些琐碎但耗神的事情一股脑交给CI自动化跑掉，评审者只需要关注真正需要人工判断的部分。
第二条我记得一开始有个不小的阻力，因为当时有些同事觉得"静态检查工具报的问题太多了，根本没人看"，然后就直接把工具给关了。我的态度是：工具报对的问题，你要想办法让工具闭嘴，而不是把工具关了。比如某个规则误报率高，那就配置白名单、调阈值，让它只在真正违规的时候说话。调好的工具链，能省下大量评审人力。
4.3 用数据暴露评审质量问题
很多团队Review做得怎么样，全凭感觉——"最近好像大家review都挺积极的"。感觉这东西是会被集体情绪带偏的。为了搞清楚真实状态，我给自己内部的实践项目加了一组简单的数据跟踪：
- PR的平均首次响应时间：从提交到第一个reviewer给出第一条评论的时间。
- 评审覆盖率：有至少一条实质评论的PR占所有PR的比例。
- 评论分布：评论数是否集中在某几个人身上，其他人是不是从不发言。
- "意见采纳率"：作者明确回复"改"或代码里落实了修改的比例。
这些数据不用做复杂的大屏看板，每周手工导一次Excel表格就能看个大概。有一次统计发现，某个组的"首次响应时间"中位数已经超过了几个小时，说明PR经常躺在挂着。拿到数据之后，我们在站会上升级了这个问题的优先级，很快就有了改善。数据不一定会让问题立刻消失，但它至少能把问题从"装看不见"变成"明摆着"。
这里我想澄清一点：数据是为了服务持续改进，不是为了给员工排名、扣绩效。一旦变成KPI考核工具，每个人就会想办法刷数据——比如疯狂发"LGTM"来把响应时间刷下去。所以这些指标只能团队内部参考，不适合做跨团队横向排名。
5. 把Review往前推：机器能做的，别用人来做
5.1 静态检查与格式化交给机器
这个话题也许有人觉得老生常谈，但我观察到很多团队的实际落地仍然很草率。他们可能有CI、有静态检查工具、有Lint规则，但因为配置粗糙或者历史报错太多，开发者已经把工具的输出当成"背景噪音"——反正错了也不会被拦，真拦了我就在代码里加个忽略。
合理的方式是逐步收敛。我不建议为了"看起来很严格"一次性打开所有规则，那样只会逼疯所有人。我常用的步骤是：
- 先只开两类规则：错误级别的（error）和风格强制的（format），其余全部warning级别；
- 给团队一个过渡期，比如两周，处理存量告警；
- 从某个时间点开始，CI里对error做硬门禁，不过不开PR合入；
- 每季度review一次告警趋势，针对反复出现的规则单独开专项清理。
一旦这一步走顺，Review能省出一大块时间。那些"少了个空指针判断""这个函数命名不符合规范"之类的低级意见，机器全给你挡了，评审人看到的是"已经干净过的代码"，体验完全不同。
5.2 基于提交历史和个人习惯的经验沉淀
这是我自己比较得意的一个小实践。我们基于内部的提交历史做了一次简单的挖掘——把过去一年里发生过线上问题的改动拉出来，做了回溯归类。结果发现，有一类问题是反复犯的：某个服务的超时时间被改短、某个接口的错误处理被吞掉、某个并发容量的初始值被写死。这些问题在Review时其实是可被识别的高频信号。
于是我们做了一个轻量级的"Review检查清单"——不是挂在Wiki上吃灰的文档，而是集成到了评论模板里：
- 这个改动是否涉及外部依赖的超时或重试？
- 异常是被抛出还是被吞掉了？吞掉的有没有日志？
- 并发场景下共享状态是否被正确保护？
- 数据库字段变更是否考虑过存量数据兼容？
清单条目不多，就十条左右，但每一条都是从真实事故里提炼的。评审人看代码时，可以对照这些信号去定向排查，比自己漫无目的地看要高效得多。
再过一段时间，我们会把这个清单做成一个自动化扫描器——用一些启发式规则去匹配代码模式，比如"吞异常但没打日志"这种模式，正则都能探个大概。机器先扫出可疑位置，评审人再人工确认。我认为这才是代码评审工具化的理想形态：机器负责掏信号，人负责做判断。
这里补充一个容易踩的坑：经验清单一定要保持"轻量迭代"，每半年删掉一些已经不重要的条目、加入新发现的高频问题。如果清单越攒越长，评审人记不住，最后就会变成摆设。
6. 实践一年之后，我沉淀下来的几条操作原则
写了这么多，最后分享几个我在实践中反复验证过的操作原则，都是即拿即用的那种。
第一条：把Review纳入工作排期，而不是空余时间。很多工程师不把评审当正经工作，往往是写完自己的代码才"顺便"去看别人的PR。这导致Review质量甚至被优先度无限压缩。我建议每天上午留出固定一小时看评审，效果比下午或者晚上好很多——上午头脑清楚，逻辑链完整，评审质量明显更高。
第二条：宁可少开几个PR，也要保证每个PR能被完整看完。这个观念和"小步快跑"不冲突。小PR的核心价值是"能被完整看完"，而不是"数量多"。如果团队一天发出来二十个PR但没有一个能被认真看完，还不如只发五个、保证每个都被认真看完。
第三条：写Review意见的时候多问自己一句：这条意见值不值得别人花时间看？如果答案模糊，宁愿先不发，等想清楚了再补。垃圾意见和假通过一样，都会消耗团队对评审机制的信任。信任崩塌一次，重建的成本极高。
第四条：评审既是技术活也是沟通活，情绪管理放在第一位。一旦评论里出现了"你怎么这么写""这不是基本常识吗"这类带情绪的词，这个评审的质量就已经开始下滑了。技术讨论建立的前提是双方都愿意理性说话。哪怕你真觉得对方犯了低级错误，也先确认一下是不是有隐性原因——很多时候背后有上下文，只是你不知道。
最后再分享一个小技巧：回头看看自己一个月前写的代码，然后用评审别人的心态去审一遍。这是提升评审能力最快的方式。你会惊讶地发现自己当初写的东西有多少漏洞和笨拙之处，这种切身体会比读任何评审指南都有效。我也是这么练下来的，现在每次看到旧代码，都跟自己说，还好当时有人在旁边帮我拦了一道。
