代码评审里没人说透的那些真问题

上周跟一个做了8年后端的朋友喝酒，他骂了半小时公司的代码评审。说每次评审跟走流程似的，大家对着提交记录挑错别字，连变量名多打了个下划线都要提，真到了逻辑有坑的地方，全装瞎。

评审的本质从来不是挑错

很多人对代码评审的误解，从把它当“代码批改作业”开始。说白了，学校里改作业是给老师交差，职场里的代码评审，是你要把自己写的东西，交到几个以后要给你擦屁股的人手里，让他们确认这东西以后不会半夜把人从床上薅起来。

麻烦在于，现在太多团队把这个事搞成了KPI。比如有的团队要求每个提交必须有3个以上评审人点头，不然合不进主干。结果呢？大家为了凑人头，随便拉两个不相关的同事点同意，连代码都没打开过。我见过最离谱的，一个写前端的同事，被拉去评审后端的数据库存储过程，他连那个存储过程用的什么方言都不知道，扫了两行就点了approve——后来那个存储过程上线3天，把用户的订单状态全搞混了，赔了小几十万的优惠券。 真正有价值的评审，从来不是找那些一眼能看到的小问题。变量名好不好，缩进对不对，这些东西交给lint工具去管就行，人要干的是工具干不了的事。比如这段循环会不会在极端数据下死循环，这个异常捕获会不会把本来要抛出去的业务错误吞掉，这个接口的参数校验是不是漏了权限校验的环节，这些才是评审该盯的东西。

评审里最容易被忽略的成本

很多团队算不清评审的账。比如一个3人小组，花1个小时开评审会，看起来好像没什么成本——但这1小时，是三个本来可以写代码、测bug、改需求的人的时间。要是这1小时全用来挑缩进、挑变量名，那纯纯是浪费。

我之前待过的一个团队，踩过这个坑。那时候我们要求所有提交必须走线上评审，每次评审人要写至少3条意见才能通过。结果大家为了凑够3条，本来没意见也要硬找。比如有人会在评审意见里写“这里的注释可以再写得详细一点”，“这个变量名可以改成更有意义的”——这些意见改了也没什么用，不改也出不了事，但提交的人要花半小时改完，再发回去重新评审，一来一回，大半天就没了。后来我们改了规则，lint能查的问题一律不准在评审里提，评审意见只写会影响功能、可维护性、安全性的问题，整个评审的平均时长直接砍了三分之二。 不过话说回来，也不能走另一个极端。有的团队嫌评审麻烦，干脆全跳过，谁写的代码谁自己上线。那更糟，我见过一个刚毕业的小孩，写了个批量删除用户数据的脚本，没加where条件，直接在生产库跑了，删了10万条用户数据，花了两天两夜才从备份里找回来。这事没那么玄，就是本来评审的时候只要有人扫一眼那个脚本，就能发现问题，结果没人看，全是代价。

好的评审从来不是对立关系

很多人把评审搞成了“找茬大会”。提交代码的人觉得评审人是在针对自己，评审人觉得提交的人写的什么垃圾东西，两边对着呛。上次我参加一个评审会，一个老员工对着新员工写的代码拍桌子，说“你这写的什么玩意，我看一眼就觉得恶心”。新员工脸涨得通红，会后直接提了离职。 说白了，大家都是来干活的，不是来比谁写的代码更牛的。你评审的时候提意见，是说“这段逻辑这里有个坑，咱们一起改了，以后少点麻烦”，不是说“你写的代码真垃圾”。我见过最舒服的评审，是提交代码的人自己会在提交说明里写清楚“这里我用了个临时方案，因为时间紧，后续要重构”，评审的人看到了，就会针对性地问“那这个临时方案会不会影响XX模块？要不要加个注释标出来？”，两个人三两句就把问题说清了，既不浪费时间，也不会伤和气。 还有个事，很多人没注意到：代码评审其实是团队里成本最低的技术传承方式。老员工看新员工的代码，能知道他哪里写得不对，新员工看老员工的提交，能知道团队里常用的技术方案是什么。要是每次评审都只挑错别字，那谁也学不到东西。我之前带过的一个小孩，刚进公司的时候写的代码全是硬编码，后来跟着看了几十次老员工的评审，自己就知道要把常量抽出来，要加异常处理，比给他讲两小时技术文档有用多了。 上周那个骂评审的朋友，后来跟我说他们团队改了规则。现在lint能查的问题不准提，评审意见只说真问题，每次评审之前，提交的人要先花10分钟把自己的代码逻辑过一遍，有坑的地方自己先标出来。现在他说，每次评审再也不用对着那些破事扯皮了，真出问题的概率也降了不少。