【译】如何用人类的方式进行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。