上周跟三个不同团队的开发吃饭，聊到代码评审，三个人的吐槽完全凑成了三个极端：有人说自己提的PR三天没人看，最后催到reviewer直接点通过连diff都没拉；有人说组里评审卡得离谱，一个变量命名要扯半小时，版本卡了三天发不出去；还有人说现在组里的评审完全变成了甩锅大会，谁写的代码被挑出问题，就是能力不行。
评审的第一笔账，根本不是代码质量
说白了很多团队从一开始就把代码评审的目标搞反了。大家默认评审是找bug的？不对，找bug那是静态扫描工具干的事，eslint、sonar扫出来的问题，比人眼漏的多十倍都不止。
那评审到底是干嘛的？大部分一线团队里，代码评审的第一属性是知识同步。比如你写了一段对接支付渠道的逻辑，你的reviewer看完了，下次你请假，他能直接接得住这个模块的改动，不用等你回来才敢动。
麻烦在于，很多团队把这个目标丢了，变成了纯粹的找茬。我之前见过一个组，reviewer每次提意见，从来不说”这段逻辑我没看懂”，直接说”你这里写的有问题”。最后提PR的人本来还想解释两句，次数多了，改代码的时候就开始对着reviewer的偏好写，不管逻辑顺不顺，只要对方挑不出错就行。
代价是什么？最后整个组的代码，全是那个最资深的reviewer的个人风格，新人根本不敢加自己的思考，三个月下来，所有人都在写别人的代码，没人敢对模块的长期设计负责。
那些看起来高效的评审流程，其实在偷偷加成本
很多团队为了让评审快，搞了很多花活。比如要求PR必须在4小时内review完，超时扣绩效。听起来很对吧？实际跑起来是什么样？reviewer手里同时挂着三个需求，开着四个会，看到PR通知弹出来，根本没时间拉本地跑，对着线上的diff页面扫两行，看到没有明显的语法错误，直接点通过。
省了时间？没省。最后代码合进去，上线出了问题，半夜三个人爬起来排查，花的时间比当初认真看二十分钟多十倍。
还有的团队搞”评审checklist”，列了几十条要求，从注释格式到单元测试覆盖率，每一条都要打勾。我上次跟一个做云服务的开发聊，他说他们组的checklist有47条，每次提PR，光对着checklist凑条目就要花半小时，有时候为了凑覆盖率，写一些完全测不到核心逻辑的垃圾测试，最后覆盖率数字好看了，真出问题的时候，测试用例一个都抓不住。
很有意思的一点是，评审的成本从来不是花在看代码上的时间，是所有人为了适配流程规则额外付出的无意义劳动。比如你本来写一段逻辑，十分钟能写完，为了过评审，要拆成三个小PR，每个PR配三段不疼不痒的注释，要找两个不同的人分别签字，要把本来可以放在一个commit里的改动拆成五个，就为了让每个PR的diff行数不超过100行——那个”PR行数不超过100″的规则，我见过太多团队用了，实际跑起来，大家为了凑这个行数，把本来连贯的逻辑拆得七零八落，后面别人找上下文的时候，要翻好几个PR才能拼起来完整的逻辑，反而把阅读成本拉得极高。
好的评审从来没有统一的标准
这事没那么玄。没有哪个流程是放之四海而皆准的。比如你做的是面向C端的活动页面，一个活动跑两周就下线，那你评审的时候根本没必要揪着变量命名扯半小时，只要核心逻辑不出错，能快速上线就行。但如果你做的是底层的存储引擎，一行代码错了可能丢用户数据，那你就算卡三个小时把每一行逻辑抠明白，都不算过分。
我之前见过一个做支付核心的团队，他们的评审规则很简单：只要是涉及资金变动的代码，必须要两个人逐行读，读完了要把可能出问题的边界场景列出来，没有通过的话，就算你测试全过也不能合。他们也不搞什么checklist，也不卡评审时长，有时候一段几百行的核心逻辑，几个人围着屏幕坐一下午，把各种极端场景掰碎了聊，最后上线半年，核心链路出的资金类问题，比之前少了很多。
慢。但算下来的成本，比之前出一次资金故障要赔的钱，少太多了。反过来，做活动的团队要是照搬这个流程，那等你评审完，活动的流量高峰都过了。
很多人吐槽评审累，本质上是把所有团队的评审规则都套成一个模子了。比如有人说评审就要严格，有人说评审就要快，其实根本没有对错，关键是你得算清楚，你现在花在评审上的时间，能帮你省掉多少未来的麻烦。
我最烦的一种评审，是那种”我是资深开发我说话算”的评审。比如你写了一段逻辑，reviewer根本不跟你聊这个逻辑在当前业务场景下合不合适，上来就说”我十年前写代码的时候就这么写过，你这个不对”。遇到这种情况，你要是跟他争，他就说你不尊重资深，你要是不吭声，最后代码改得一塌糊涂，出了问题他还会说”当初我就提过意见”。说白了，这种评审根本不是技术协作，是职场权力的延伸。
上次跟那个做云服务的开发吃饭，他说他们组现在改了评审规则，每次评审之前，提PR的人要先花五分钟跟大家说清楚，这段代码改了什么，为什么要改，可能的风险点是什么。然后reviewer的意见，不能只说”这里不对”，必须说清楚”这里如果遇到XX场景，会出XX问题”。就这么两个小改动，之前大家吐槽的评审扯皮问题，少了一大半。