我做过大概不到一百次评审，说多不多，说少不少。但最近一次评审让我彻底改变了看法。我们团队用GitHub PR做评审，有个改动是重构一个订单超时处理的模块，我花了一个多小时仔细看，评论了两处变量命名和一个非常不优雅的缩进。结果合上去第二天线上出了个bug，原因是原来的重试条件被不小心反转了。我在评审时根本没注意，因为注意力被那些表面问题吸走了。这让我感觉很糟，但也让我开始反思评审到底应该看什么。那之后我发现，不只是我，几乎所有人都把评审当成了一个找茬的游戏，而不是理解代码的方式。
我们团队规定所有PR必须至少一个人approve，于是经常出现“LGTM”但实际没人仔细看的情况。Code review变成了流程上的“必须完成的任务”，而不是技术上的交流。很多人在评审时只关注代码风格，因为那是最快能说上话的部分。有个很讽刺的现象：越是新手，越喜欢点评命名和格式，而真正的逻辑错误常常在讨论区鸦雀无声的时候悄悄溜过去。这不是某个人的问题，是整个体系把评审推向了“表面文章”。而且很多公司还把评审数量当作绩效指标，导致大家为了review而review。
我们试过结对编程，两个人一台电脑，即时讨论。那确实是另一种体验，但它对时间要求太高，不可能所有代码都结对。也试过用Gerrit那种逐行评论的静态评审工具，感觉像是在法庭上辩论，太正式，反而让人不愿意说话。后来我意识到，评审的本质不是“找bug”，而是“建立对系统的共同理解”。如果你看一段代码，没有提出问题，但你和作者讨论出了某个设计决定的原因，那比找到三个unused variables有价值得多。所以真正好的评审，应该是在PR描述里写清楚背景和意图，而评审者的问题应该多问“为什么”，而不是“为什么不用这个”。
我们现在的做法是：把评审分成了“正式审批”和“异步讨论”两层。正式审批只检查是否满足安全清单，比如有没有日志、有没有测试、有没有数据库迁移。异步讨论则是任何时候都可以在IM里贴一个diff链接，大家随便聊，不需要approve。这样做的效果是，评审的压力变小了，但讨论变多了。我还建议团队把“是否理解了这段代码”作为评审完成的标志，而不是“是否approved”。当然这很难度量，但至少能让评审回到它应该有的样子。毕竟软件工程里，一个不关心人的流程，最后都只会被敷衍。技术问题大多不是技术本身造成的。
