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

正文

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

第一篇文章中,我介绍了不少基础的东西,因此我更推荐从那里开始读。固然若是你没有耐心,那么这里用一句话归纳一些:一个好的代码评审者,不只须要找出代码中的 bug,还须要提供认真负责的反馈来让团队伙伴获得能力上的提高。网络


我最糟糕的一次 Code review

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

Review 的经历

当我收到 Mallory 的第一份变动列表,里面的代码是很粗糙的。她以前彻底没有写过 Python 代码,并且她的代码是基于一个我维护的笨重的老系统上开发的。post

我很尽职地记录下了全部我找到的问题,一共有 59 个。按照其它 Review 文章的标准来看,我作得很不错。我找到了如此多的错误,因此我确定是个优秀的评审者。debug

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

我看到这些,心情有些恼怒,因此又写了一轮新的评论。个人语气很专业,可是不可避免的有些火药味。“你能解释一下,为何咱们须要对于错误的输入会产生不肯定的行为吗?”正如你所猜的那样,Mallory 的回复比以前更执拗了。3d


恶性循环

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

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

我读了代码以后,个人心剧烈地跳动,由于真的被她惹怒了。我马上开始奋力敲击个人键盘,指出她既没有按照个人要求做出修改,也没有用任何理由说服我批准这个变动列表。视频

咱们就这样毅种循环(误)了整整三个星期,而代码几乎没怎么改动过。


介入

幸运的是,咱们团队里最高级别的工程师,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。

相关文章
相关标签/搜索