上周在园区咖啡馆撞见两个后端工程师拍桌子。
原因是一个人觉得另一个提的PR里连个空值判断都没加，另一个觉得对方吹毛求疵，耽误上线时间。
我坐邻桌听了十分钟，两边说的都有道理，也都没说到点子上。
代码评审不是挑错大赛
很多团队把这环节搞成了低级错误擂台赛。谁找的变量命名不规范多，谁的评审优先级就高。
我之前见过一个小组，评审的时候有人揪着一行返回值的命名少了个复数后缀叨叨了二十分钟。那行代码是写在内部管理后台的导出接口里的，半年都不会有人主动翻。
说白了，大家都是来干活的，不是来考校对方标点符号用得对不对的。
真要是为了挑错而挑错，最后没人愿意认真写PR。大家都把精力花在怎么把变量名写得符合规范，反而没人关心这段逻辑会不会在高并发下崩。
很多人对评审的误解从一开始就偏了。它不是上线前的质检关卡，更像一起搭伙走夜路的时候，大家互相照个手电筒。你走的那段路有没有坑，旁边人帮你瞅一眼，仅此而已。
那些藏在评审记录里的隐性成本
我见过太多团队为了赶版本，直接把评审环节砍了。要么提PR的人自己给自己点合并，要么拉个群丢个链接，十分钟扫一眼就过。
省下来的时间确实多。上线能快大半天。
但代价呢？
去年听一个做本地生活的工程师聊，他们赶春节版本的时候，连续三周所有PR都是提交者自主合并，连个过目的人都没有。结果节后流量上来，有个商家结算的分支逻辑漏了判断，系统自动给商家多打了三倍的款。最后整个团队连着加了一周班，对账、改代码、给商家发通知补款，算下来花的时间是之前评审总耗时的二十多倍。
省下来的两小时评审时间，最后要花两百小时去擦屁股。
麻烦在于，这种代价从来不会在项目排期表上体现。你看不到哪个任务条写着”因为没评审，后续要多花多少工时”。等真出问题的时候，所有账都算在”线上紧急故障”的条目里，没人会记得当初是为了省那点时间偷的懒。
还有更隐蔽的代价。没人敢碰的屎山，就是这么堆出来的。你写的代码没有经过第二个人的眼睛，三个月后你自己都忘了为啥要写那个绕了三层的if分支。后来接你活的人看不懂，又不敢随便改，只能在上面继续叠逻辑，越叠越乱。
好的评审从来不是单向挑错
我最烦两种评审场景。
一种是资深工程师对着新人的PR一通骂，说”你这写的什么垃圾”，半句话都不说怎么改。另一种是评审的人根本没点开过代码，最后随手点个”同意”，连个评论都没有。
前者是把评审当自己刷权威的场子，后者是把评审当凑数的流程。都没用。
之前在一个技术开放日见过一个工程师分享他们团队的评审规则。提PR的人自己要先写清楚三件事：这次改动涉及哪些模块，为什么要这么改，哪部分逻辑自己拿不准需要重点看。评审的人收到通知后，尽量在当天给出反馈，意见要具体，不说空话。
比如看到循环逻辑有问题，就说”你这段遍历的时间复杂度是O(n²)，后续用户量涨上来可能会有响应超时的问题，你可以试试用哈希表预存值”，而不是说”你这写法不对”。
没人喜欢被否定。但所有人都愿意改能让自己少踩坑的代码。
上次碰到个刚毕业的小孩提的PR，里面有个异步任务的优化思路特别巧，就是写法绕了点。负责评审的资深工程师没有直接打回去让重写，而是留了个评论，说这个思路他之前踩过类似的坑，用队列削峰的话能少写三十行，还能避免重复消费的问题。小孩改完之后，不仅那次评审过了，后来还主动把这个优化思路用到了别的三个模块里。
这才是评审该有的样子。不是谁压谁一头，是大家互相换个角度看问题。你想到的边界情况我没考虑到，我踩过的坑你刚好能避开。
代码评审这事儿，没那么玄。
不需要你把每一行代码都改到教科书级别的优雅，也不需要你搞一堆复杂的checklist卡着所有人的上线节奏。
就是一群一起干活的人，互相搭个手。别让后面接代码的人踩坑，也别让自己写的东西最后变成没人敢碰的烂摊子。
够了。