【译】如何用人类的方式进行 Code Review（二）

这是我文章的后半部分，关于如何在 Code review 中进行良好的沟通，避免陷入一些潜在的陷阱。这里，我会着重于介绍一些技巧，让你的 Code review 能够顺利完成，避免磕磕碰碰。

在第一篇文章中，我介绍了很多基础的东西，所以我更推荐从那里开始读。当然如果你没有耐心，那么这里用一句话概括一些：一个好的代码评审者，不仅需要找出代码中的 bug，还需要提供认真负责的反馈来让团队伙伴得到能力上的提升。

我最糟糕的一次 Code review

我最糟糕的一次 Code review 经历，是和一个叫 Mallory 的前同事有关。她早我几年加入公司，但最近才转到我的团队来。

Review 的经历

当我收到 Mallory 的第一份变更列表，里面的代码是很粗糙的。她之前完全没有写过 Python 代码，而且她的代码是基于一个我维护的笨重的老系统上开发的。

我很尽职地记录下了所有我找到的问题，一共有 59 个。按照其它 Review 文章的标准来看，我做得很不错。我找到了如此多的错误，所以我肯定是个优秀的评审者。

过了几天后，Mallory 给了我一份更新过的变更列表，已经针对我之前的 Review 修改过了。她修复了一些简单的问题，比如拼写错误、变量重命名等等，但她拒绝修改任何更高层次的问题，比如她的代码对于错误的输入会产生不确定的行为，还有她的一个函数嵌套了 6 层逻辑。她拒绝修改这些东西，轻蔑地解释说，这些东西不值得花工作时间去修复。

我看到这些，心情有些恼怒，所以又写了一轮新的评论。我的语气很专业，但是不可避免的有些火药味。“你能解释一下，为什么我们需要对于错误的输入会产生不确定的行为吗？”正如你所猜的那样，Mallory 的回复比之前更固执了。

一周后的星期二，Mallory 和我依然停留在第四轮相同的 Review 上。我前一天晚上提交了我的新一轮评论，但实际上我是故意拖了一天才提交的，因为当她读这些评论时，我不想跟她呆在一个屋子里。

整个早上，我一直觉得胃不舒服，因为我恐惧又要开始新一轮 Review。当我吃完午饭回来时，发现 Mallory 已经提交了新的代码，但她已经不在位置上了，我估计她也不想呆在我身边看着我审查这些更新。

我读了代码之后，我的心剧烈地跳动，因为真的被她惹怒了。我立刻开始奋力敲击我的键盘，指出她既没有按照我的要求作出修改，也没有用任何理由说服我批准这个变更列表。

我们就这样毅种循环（误）了整整三个星期，而代码几乎没怎么改动过。

幸运的是，我们团队里最高级别的工程师，Bob，帮我们打破了这个循环。他刚刚休完长假回来，惊讶地发现我们两个正在把 Code review 相互扔来扔去。他意识到这是一个僵局，所以请求我们让他接管这个 Review，我们都同意了。

Bob 在最开始时，先让 Mallory 创建了几个新的变更列表，把我们俩之前争吵从来没有提到过的两个小型库分隔开，每个大概 30 - 50 行。Mallory 做完之后，Bob 立刻批准了。

然后，Bob 回到主要的变更列表上来，这个列表现在已经被裁剪成 200 行左右的代码。他给了一些小建议，Mallory 响应修改了。再然后，他批准了这个变更列表。

Bob 的整个 Review 就花了两天。

沟通很重要

你或许已经发现，这儿产生的冲突其实并不是关于代码的。Code review 里提出的问题都是合情合理的，它们也明显可以被沟通能力强的团队成员解决。

这是一个很不愉快的经历，但我很高兴能回想起来，因为它让我重新思考了我的 review 方法是否恰当，并从中找到可以改进的地方。

下面，我会介绍一些技巧，以避免类似的不愉快经历。然后我会回到 Mallory 这个例子上，解释为什么我之前的方法是不好的，而为什么 Bob 做得那么好。

旨在把代码改进一到两个级别

即使你的团队伙伴，理论上，想找到任何机会来让他们的代码变得更好，但他们的耐心也依然是有限的。当你经过一轮又一轮的 review 之后，依然不肯批准变更，他们的心情会变得很沮丧，因为你总是在不停地想各种方法来改进他们的代码。

我个人会把代码分几个级别，从 A 到 F。当我收到一份评分为 D 的变更列表，我会尝试帮助作者把它改进到 C 或者 B-。不是完美的，但足够好了。

当然，把一份 D 评分的代码改进到 A+ 在理论上是可能的，但它可能会需要八轮 review。最后，代码作者会怨恨你，并且未来再也不给你任何 review 的机会了。

你可能会想，“如果我批准了 C 等级的代码，那最后代码库里不就全都是 C 等级的代码了吗？” 其实不然，我发现，如果我帮助团队成员从 D 改进到 C，那么他们的下一份变更列表就会从 C 等级开始。几个月后，他们给我的 review 都是从 B 等级开始了，这些改进之后就会变成 A 等级。

当然有一个特例，那就是 F 等级，这个等级是保留给一些写得太烂的代码的，一般是功能不正确，或者写得太错综复杂，以致于你无法判断代码的正确性。你驳回它的唯一理由应该是，它经过了几轮 review 之后，依然没有任何改进。这时，请参考下面的关于僵局的部分。

对于相同的问题，有限度的反馈

当你发现了作者很多相同的问题，不需要把每一处都标明，你肯定不想把同样的评论重复 25 次，代码的作者也不想读 25 次相同的评论。

对于同样的问题，只需要重复注明 2- 3 次，然后剩下的，就直接评论说让作者修复类似的问题就好了，而不是去注明每一处问题。

遵守 review 的范围

有一个我经常见到的反模式，那就是评审者会评论变更列表附近的一些代码，并要求作者修改它们。作者修改后，评审者通常会觉得代码确实更好了，但和别处的代码有一些不一致，于是又要求进一步的更改，以及再进一步的更改。最后让变更列表扩展得很大，包括了很多不相关的东西。

如果一只饥饿的小老鼠出现在你的家门口，你可能会想给他一块饼干。如果你给他一块饼干，它又会想要一杯牛奶。然后它会想要一面镜子，以确保它的胡子上没有粘上牛奶，然后它会要一把剪刀给自己剪剪胡子...
—— Laura Joffe Numeroff, If You Give a Mouse a Cookie

所以有一条准则：如果变更列表没有涉及到这一行，那它就是超出 review 范围的。

下面就是一个例子：

即使你失眠了一整晚，被这个魔术数字和荒唐的变量名所困扰，它也是超出范围的。即使写下这一行代码的人就是本次 review 的作者，它也是超出范围的。如果它真的非常糟糕，那也请你提一个 bug，或者提交你的修复，但不要把它放到本次 review 的范畴中来。

当然有个例外，那就是当变更列表影响了周围的代码时，比如：

这个例子里，需要让作者把 ValidateAndSerialize 重命名为 Serialize。虽然这超出了变更列表的范围，但它会导致不正确的命名。

当我没发现问题，但发现了范围外的某个很容易修复的问题时，我会不遵守这个规则。在这种情况下，我会明确地表示，作者可以完全无视它。

尝试切分体积很大的 review

当你收到了一个约 400 行代码的变更列表，你应该鼓励作者把它切分到更小的块。超过得越多，你就越应该打回这个变更列表。我个人拒绝 review 任何超过 1000 行的变更列表。

作者对于 “切分变更列表” 这件事可能会有些怨言，因为这是个很乏味的任务，所以作为评审者的我们最好为作者划出要切分边界，以减轻他们的负担。最简单的一种情况是，变更列表涉及到多个文件，这时便可以按文件切分为多个更小的变更列表。当然也会有更复杂一些的情况，这时也许需要找到抽象层次比较低的函数或者类，然后要求作者把它们移动到另一个变更列表里，等另一个变更列表合并之后，再回来看现在的这一个。

当代码写得很糟糕时，就更应该要求切分。因为这时 review 的难度随着代码的长度呈指数级增长。Review 多个 300 行的变更，比 review 单个 600 行的变更，要好得多。

真诚的表扬

大多数评审者都只关注代码中的错误，但忘记了 review 也是一个促进积极行为的好机会

比如，你正在 review 的这个作者是个文档写得很挣扎的人，但你却看到了一条清晰、简洁的注释，这时请表扬他一下。当你告诉他们什么做得对时，他们会进步得更快，而不是等着他们犯错才告诉他们。

你不需要刻意地去表扬作者，每当我看到变更列表中有任何亮点，我都会告诉作者：

• “我没想到还有这个 API，它用处很大！”
• “这是个很优雅的解决方法，我之前还没想到可以这样做。”
• “这个函数重构得很不错，它现在简单得多了。”

如果作者是个最近才加入团队的萌新，他们在 code review 的时候可能会感到紧张和焦虑。为了缓解这种情绪，你需要真诚地表扬他们，证明你是支持他们的同伴，而不是残酷无情的代码守门员。

当剩下的都是些小问题时，直接批准

有些评审者会有一个错误的观念，他们总会一直不肯给出批准，直到他们看到每一条评论都得到了修复。这增加了不必要的工作，也浪费了作者和评审者的时间。

当有下面这些情况时，可以直接批准变更：

• 你没有更多的评论想写了
• 剩下的评论都是些无关紧要的小问题，比如变量命名、拼写错误
• 剩下的评论都是 “可选的”（记得明确地标明这些评论不是一定要修复，这样你的小伙伴才不会认为一定要改掉这里才能让你批准）

我之前看到过有些评审者一直不肯给出批准，因为作者在最后的评论后就没再有任何回应了。请不要这样做，这会让作者觉得你认为他们连加一个标点符号都应该被监视着。

当还剩一些很重要的问题没修改时，直接给出批准可能会很危险。根据我的经验，大约有 5% 的概率会发生这样的事，要么作者误解了最后一轮评论的意思，要么他完全没看到。为了解决这种情况，我会去检查一遍要批准的变更。如果真的是因为罕见的沟通不畅，我要么会继续跟进作者，要么会自己创建一个新的变更来修复问题。在 5% 的情况下增加少量的工作，要好过于，在另外 95% 的情况下加入更多不必要的精力和拖延。

主动处理僵持状态

在 code review 中，最糟糕的情况就是僵持：如果没有进一步更改的话，你不想批准这个变更列表，而代码的作者拒绝修改它。

下面这些指标，表示你正在走向僵持状态：

• 讨论的语气变得越来越有敌意和火药味
• 你的每一轮评论都不是自上而下的了
• 你回复的评论异常之多

直接面对面交流或者通过网络视频。文字交流最后会让你忘了你对话的是一个真正的人，很容易让你想象你的同伴是一个来自无能与固执之地的人。面对面的交流将会打破这个魔咒。

考虑设计阶段的审查（design review）

可能会进入僵持状态的 code review 可能在更早的时候就有一些征兆了。你们现在是不是在一些事情上争吵，而这些事情本应该在设计审查时讨论的？你们有设计审查吗？

如果分歧的根源是高层次的设计问题，那就应该是由更大的团队来权衡利弊，而不是交给 code review 的两个人。你应该和作者在 review 中讨论，同时开放给团队其他成员，以设计审查的形式让他们也进来讨论。

让步或者升级

你和你的同伴在 review 中僵持得越久，就越有害于你们之间的关系。如果问题一直没有得到解决，那么你最好选择让步，或者把问题升级。

学会衡量给出批准的成本。当你总是接受低质量代码时，你的软件质量当然不可能好，但当你和你的团队伙伴痛苦地工作，以至于你们无法在一起愉快地合作时，软件质量同样也不会有任何提高。这个时候应该想想，如果你批准了变更列表，结果会有多糟？这些代码会破坏重要的数据吗？它是一个后台进程，并且只有当最坏的情况发生时才需要开发人员对它进行 debug 吗？如果更偏向于后者，那么可以考虑批准变更，以便让你和你的团队伙伴继续工作。

如果你不想做出让步，请告诉作者，将这里的争议升级到团队的管理者或者技术负责人那里，把 review 重新分配给另外的评审者。即使之后得出的结果和你之前的意见相悖，也要接受它。继续固执下去只会导致更不好的结果，让你看起来很不专业。

脱离僵局之后

一次棘手的 review 中的争吵，真正的重点很可能并不是代码，而是人和人的关系。如果你陷入了僵局或者即将陷入僵局，并且不去解决那些潜在的冲突，那么这种情况会一直发生下去。

• 与你的上级讨论这种情况
  • 如果团队中产生了冲突，你的上级应该会知道。也许是因为代码作者本来就是个很难合作的人，也许是因为你们只是用了相互了解的方式在解决当前的状况。一个好的上级会帮你们解决这些情况。
  • 如果可能的话，相互之间停止代码评论几个星期，直到两边都冷静下来。
• 学会解决冲突
  • 有一本书叫 "Crucial Conversations" 很不错。虽然它给出的建议似乎都是些常识性的东西，但当你作为一个旁观者来看冲突时，就会发现它分析冲突的方法真的很棒。

重新思考我最糟糕的那次 Code review

还记得我和 Mallory 的那次 code review 吗？为什么我的 review 花了整整三周，并且成效甚微，而 Bob 仅花了两天就搞定了？

我做错了什么

这是 Mallory 在我们团队的第一次 review。我之前并没有考虑到她可能会感觉自己受到评判，产生戒备心。我应该在最开始的时候给出一些高层次的评论，让她不会因为大量的评论而感到挫折。

我应该更好地向她表明，我的工作不是去阻碍她的工作，而是帮助她变得更好。我应该给出更多的代码示例，以及在她的变更列表中给出更多的积极因素。

我的自尊心也过多地影响了这次 review。要知道我花了一整年的时间，才把这个老系统修复到健康状态，突然有了一个新人开始在这个系统上做些事情，而她就不再需要考担心这些我曾经担忧的问题了吗？我认为这是一种冒犯，也就是这种态度产生了副作用。我应该在所有的评论上都保持一个客观的态度。

最后一点，我让这个僵局持续太久了。在几轮 review 过后，我们都应该很清楚，我们没有任何有意义的进展。这时我应该主动做出改变，比如主动当面交流，以解决深层次的冲突，或者把它升级到我们的上级那里。

Bob 哪些地方是对的

Bob 第一步把 review 切分的做法非常有效。这个 review 已经花费了痛苦的三周时间，忽然，其中的两块代码被合并了。这让 Mallory 和 Bob 都感觉到激励，因为它产生了向前的动力。尽管剩下的代码里依然存在问题，但它变成了一个更小的，更好管理的变更列表。

Bob 没有尝试在 review 的过程中把代码改造成完美无缺的，他可能已经意识到那些代码里我觉得无法忍受的问题，但他也同样意识到 Mallory 会待在我们团队一段时间。对于某些特殊情况的灵活处理，让他可以在长时间内帮助 Mallory 提升质量。

在我发表这篇文章的前半部分之后，有些读者对我鼓励的沟通风格产生了一些质疑。有人觉得它太屈尊卑微了，有人觉得这种风格太委婉了，会产生误解。

这样的反馈是合情合理的。对于同样的一条评论，有人可能会觉得它很粗鲁无礼，而有的人却会觉得它很简洁高效。

当你在 review 代码时，你会面临很多选择：该关注什么，怎么写反馈，什么时候给出批准。对于这些选择，你是否遵循我的建议其实不重要，只要你记得有这些选择就好。

没有人可以给你一个完美的 code review 指南。什么是最好的技巧，取决于代码作者的性格、你和他们的关系还有你们团队的文化。不断思考你的 code review 会产出什么结果。当你遇到任何紧张形势时，先退一步思考为什么会发生。关注你 review 的质量。如果你觉得代码质量很难提升到符合你的标准，应该思考 review 的那些过程阻碍了你，以及如何解决它们。

最后祝好运，希望你能用人类的方式进行 code review。