代码评审从来不是点个同意那么简单

上周跟着跑研发工位访谈，看到个开发对着屏幕皱了快十分钟眉。他提交的PR挂了三天，评审意见攒了17条，最后一条是“这个变量名我觉得不够表意”。 他差点把键盘扣了。

被误解成挑刺的评审现场

现在太多团队把代码评审搞成了两个极端。一种是纯走流程，PR弹窗弹出来，扫三秒就点同意，连改了哪几行都没仔细看。之前碰到个团队，线上出了个隐蔽的内存泄漏，查了半天才发现，那段出问题的代码，当时三个评审人全秒点了同意，连半句评论都没留。另一种是彻底走偏成批斗会，评审人拿着放大镜找不痛快，你少写个空行、变量名用了a和b、循环里多打了一行调试日志，都能揪出来写三行意见，完全不管这段代码是要赶在大促节点上线的应急逻辑。 最离谱的一次，我看到个资深开发给新人写的登录接口提意见，说你这里的密码加密算法为什么不用我们三年前废弃的那个？新人问理由，他说“我觉得那个更安全”。翻了翻内部文档，那个废弃算法去年刚出过两次高危漏洞，连维护分支都早就删干净了。

说实话，很多人搞了好几年代码评审，根本没搞懂这件事的核心目的是什么。

评审该抓的从来不是格式问题

格式问题本来就不该占用评审人的精力。Pre-commit钩子、lint工具、格式化插件，把所有能机器校验的规则全扔给自动化流程，别让人当人肉格式检测器。之前跟个研发负责人聊，他们团队之前每周花在调整代码格式上的评审时间超过12小时，后来把所有格式化规则配到CI流程里，不符合规范的代码根本弹不出评审申请，一下子省出来的时间，全用来查真正的风险点。 那真正要抓的是什么？是这段代码改了之后，会不会把原来的多分支判断搞混了？是新增的数据库查询有没有走对索引？是异常分支触发的时候，会不会把带敏感信息的请求参数直接抛到前端日志里？是做缓存更新的时候，有没有考虑到多实例部署下的缓存击穿问题？

我见过一次很典型的取舍。某团队做订单超时自动取消的逻辑，一开始新人写的方案是，用一个定时任务每分钟扫全量超时订单，代码写得很快，测试用例也全过了。评审的时候没人揪着变量名好不好看，全在算代价：现在每天10万单的时候，扫一次全表要8秒，等单量翻到100万，这个定时任务跑一次要多久？会不会把数据库连接池占满，拖垮正常的下单接口？最后大家凑出来的方案是，把超时消息扔到延迟队列里，到点再触发取消逻辑，虽然多写了两百行消息消费的兜底代码，但算下来，哪怕单量翻十倍，数据库的压力也不会涨。 这事没那么玄。代码评审的核心从来不是证明写代码的人水平不行，是把可能要花几个小时甚至几天排查的线上问题，在代码还没合并到主分支的时候就掐掉。

没人敢说真话的评审才是真的成本

很多团队的代码评审最后变味，根本原因是层级感太重。如果提交评审的是个高职级的技术负责人，下面的小开发谁敢提反对意见？之前碰到个团队，技术负责人写的一段核心交易逻辑，提交评审之后，三个评审人全写的“看了没问题，同意合并”。结果上线之后，因为边界条件没考虑全，大促当天漏算了三百万的交易金额。事后复盘才发现，那三个评审人当时都看到了那个逻辑漏洞，但没人敢说——毕竟对方是直属领导，提了意见万一被觉得挑事怎么办。 更麻烦的是，很多团队会把评审意见的数量算到开发的绩效里，你提的意见越多，说明别的开发代码写得越差，最后大家都和和气气，你好我好大家好，PR全秒过，等线上炸了再一起背锅。 我之前在个五人小团队待过，没人讲职级那套，PR提交之后，谁有空谁就评，看到问题直接说，哪怕是团队最资深的开发写的代码，新人也能直接提“你这里没判空，用户传个空值就会报错”。那段时间我们的线上故障率是我待过的所有团队里最低的，不是因为大家代码写得有多完美，是因为没人怕说真话得罪人。后来团队扩到二十多个人，开始搞层级，搞和绩效挂钩的评审积分，用着同样的工具，走同样的流程，故障率反而翻了三倍。 很多团队天天喊着要提升工程效率，要搞研发效能，花大价钱买各种管理工具，搞各种流程培训，偏偏最能提效的代码评审环节，全是在做无用功。你想啊，一次线上故障的平均排查成本，够你把所有浪费在挑格式问题上的评审时间省下来，多写几个单元测试，多做几次接口压测，何苦呢？ 别把代码评审搞成什么高大上的方法论，什么“评审黄金法则”“顶级团队的评审秘籍”，全是虚的。说白了，就两个底线：第一，别让机器能查的事浪费人的时间；第二，别让层级关系堵了说真话的通道。剩下的，不管你是每次评审花十分钟过核心逻辑，还是花半小时拉会逐行看，只要真的能把风险挡在合并之前，就是有用的。 别等线上报警响了，才想起当初那个三秒点下去的同意，到底有多轻。