原文标题:The gentle art of code review
如果程序员彼此合得来,他们就会玩一种叫做 "结对编程 "的游戏。如果不喜欢,这个游戏就叫做 "同行评审"。 —— —Anna Nachesa - 安娜·纳切萨
“野蛮”、“踩雷”、“令人沮丧”(“Brutal”. “Minefield”. “Frustrating”.)所有这些词在人们描述代码审查经历时的投票率都高于我在本文中使用到的词。
这是为什么呢?为什么代码审查如此糟糕,我们能做些什么?
代码审查是软件开发的必要组成部分,但是如果这个部分做得不好时,就会导致人大量的人与人之间的摩擦、焦虑、冲突,浪费时间和精力,而大多数情况下确实也是如此。
我们如何才能做得更好呢?我们是否能够以善意、温和、谦逊和同情的态度给予和接受代码评审?什么样的代码注释可以增加代码的价值,并与我们的同事建立信任和安全感?我们如何突出积极的一面,应对受伤的虚荣心,并在不树敌的情况下表明观点或立场?
代码变更的生命和时间线
让我们先来谈谈什么是代码审查以及何时进行代码审查。我们将使用我的三个朋友 Aisha、Bryan 和 Chiku 作为我们的例子。
通常的工作流程是这样的:Aisha 编写一些代码(或者 Aisha 和 Bryan 结对编写一些代码),然后将代码送入版本控制,通常是在一个分支上。Aisha 现在可以进行 pull request,也称为合并请求 merge request,当 Chiku 批准后,她的所有的改动(代码变更集)与主线合并。
另外,一有些团队直接将更改推送到主线,有时也会定期从主线拉取变更到发布分支,以便在必要时构建 release 版本发布。无论哪种方式,通常都需要对代码的变更进行审批,审批人可能是更高级的人,但通常只是由团队中的其他人。
作为批准的一部分,Chiku 应该审查代码,其中“审查”至少意味着“阅读”代码,但也有可能会提出意见或询问有关代码的问题。随后,Aisha,Bryan 和 Chiku 三人进行交流,通常会做一些进一步的更改,最终批准合并。
结对评审
请注意,我们并没有说这个对话必须通过版本控制软件本身进行:例如,作为 GitHub 拉取请求的注释。实际上,最好的方式是通过交谈,与那些小文本框相比,直接对话是一个高效的交流渠道。
换句话说,代码应该是需要结对审查的,也应该要结对编程的。例如,Aisha 和 Chiku 可以共享屏幕,一起查看代码,提出问题,提出建议,解释歧义,并强调潜在的问题。
通过语音同步进行这样的交流要快得多:
CHIKU:
int向下转换安全吗?如果值大于 32 位呢?AISHA: 不可能,因为如果你看这里,它来自……
CHIKU: 哦,我现在明白了。如果我们在这里加一个评论,只是为了明确这一点?
AISHA: 当然...(打字)
我看了一下,这只花了 15 秒钟,但如果他们还得通过 GitHub 上的评论再绕上几圈,那合并工作可能就要推迟到周二了。另外, Aisha 和 Chiku 可能还是好朋友,即使是最好的关系,碰到冗长的代码审查辩论,这种关系也可能会受到影响。
语音语调甚至面部表情都会传递出许多额外的信息,而这些信息在其他情况下是会丢失的,而且不知何故,人们在面对面时总是比来回打字时更加温和、更加尊重对方。
也许正是出于这个原因,有时候你会遇到一些人,他们坚持要把讨论变成不同步、纯文字的方式。抵抗这种方式的最简单方法就是不要加入。如果他们通过 GitHub 或电子邮件向您发送对代码的评论,你就只需亲自联系他们,并要求进行面谈。
如果他们还是不愿意,那就这样吧。很明显他们不想和你交谈;他们只想说,而你却在一旁欣赏地听着。这不可能有什么帮助,所以你应该完全绕过他们,找一个对真正的代码对话感兴趣的人。
随着时间的推移,你会与那些能与你“结对审查”代码的人建立起一个有用的关系网络,你的代码也会因此变得更好(他们的也是)。
使用注释增加代码的价值
不过,像上面这样的结队审查并不总是可能的,所以当代码审查是以文本形式进行的时,这意味着我们需要更加注意我们说的是什么以及如何说。我们从“说什么”开始吧。
将代码审查看作是为现有代码增加价值的过程是一个不错的思路。因此,您打算发表的任何评论最好都能做到这一点。以下是一些在审核他人代码时可能会产生的不同反应的措辞和框架:
- 不是我的风格。每个人都有自己的风格:他们最喜欢的命名方式、代码格式化方式和语法表达方式。如果这段代码不是你写的,那么它就不会符合你的风格,但没关系。你不需要对此发表评论,改变代码以符合你的风格不会增加代码的价值。因此,别管了。
- 不明白这是干什么的。 如果你不确定代码到底写了什么,那就是你的问题了。如果你不知道某段语言语法是什么意思,或者某个函数是什么作用的,就去查一下。作者只是想让他们的工作完成,而不是教你如何编程。
- 不明白它为什么会这样。 另一方面,如果你不能弄清楚代码为什么这么写,你可以这样提问:“我不太清楚这样做的意图。是不是有什么是我没看到的地方”。通常是有的,所以要问清楚,而不是把这标记为“错误”。
- 还可以更好。 如果代码基本上没问题,但你认为有更好的写法,而不仅仅是风格问题,那么把你的建议变成一个问题。“如果写成......会更清楚吗?你认为
X是...更合理的名称吗?重复使用这个变量会更快吗,还是这个并不重要?” - 需要考虑的问题。 有时,你觉得有一个会产生帮助的想法,但你并不很确定。你会想:也许作者已经想到了这个想法并舍弃了它,或者他们只是没有想到。但你的建议很容易被解释为批评,所以要试探性地、温和地的提出:“我突然想到,在这里使用
sync.Pool可能会略有改进,但也许这只是矫枉过正。你怎么看呢?” - 不要认为这是对的。 如果在你看来,觉得代码不正确,或者不应该存在,或者缺失了一些应该存在的代码,那么请提出问题,而不是指责。“我们通常不是要检查这个错误吗?有什么理由可以不在这里做一个检查吗?” 万一如果你错了,你就给自己留下了一条优雅的后路。如果你是对的,你已经巧妙地表达了自己的观点,而且不会树敌。
- 漏掉了一些东西。 代码就其本身而言没有问题,但是有些情况作者没有考虑到,或者一些重要的问题他们忽略了。使用“是的,而且......” 这种句式的技巧,例如:“这在一般情况下看起来很好,但我想知道如果这个输入量非常大,会发生什么情况?是否应该......?”。
- 这肯定是错误的。 作者只是说漏了嘴,或者有你知道一些他们不知道的事情。这是你以应有的善意和谦逊启发他们的机会。不要只是喋喋不休地指出问题所在;花点时间仔细、优雅地作出回应。再次,使用问题和建议。例如:“看起来我们在这里记录了错误,但代码还是要继续。如果结果是
nil,这样能真的保证安全吗?你觉得在这里返回错误信息怎么样?”
强调积极的一面
我们经常认为代码审查是一个指出错误的过程,但也有一个很好的机会来表扬正确的好机会。实际上,您的审查应该以描述你喜欢的代码的一切开始。“这代码组织得真的很好,我喜欢你使用 X,Y 和 Z 为读者提供额外的上下文。函数名起的很棒,这似乎是一个不错的的 API 接口”。
一勺糖能让药更好地下咽,即使你不得不给别人难以启齿的反馈,你也总能找到一些好话。以正面引导、委婉的方式、尊重他人的评论和建议,最后用鼓励的话语结束评论:“干得好!这工作做的真的很不错。”
但是,如果你正在审查比你更资深的人的代码,那么告诉他们他们做得很好可能并不合适。他们可能会觉得,无论对错,你还没有资格去作出评价。相反,你可以这样说:“我真的从中学到了很多东西,谢谢您给我机会阅读和学习的机会。”
当你的虚荣心受到伤害
代码审查会打击你的自尊心,尤其是在审查方式不恰当的情况下。
你全心全意地投入一项工作,满怀希望地把它送出去,希望它能被赞誉为一项胜利,结果它回来时却被批得体无完肤。
你感到受伤,你感到愤怒和怨恨,你很想反击。"哦,是吗?那我告诉你,伙计......"
不用说,这不是个好主意。你的评论者几乎肯定是出于好意,即使他们并没有这样说。如果他们不是出于好意,而是有意对你不屑一顾、伤害你,那么他们肯定是个混蛋。
但这个世界充满了混蛋。你的工作不是把他们都变成正直的人,当然你也不会因此得到报酬。
与其这样,不如忍气吞声,继续前进。他们的反馈可能比你一开始准备承认的要真实得多,即使是以一种不必要的粗暴方式表达出来的。
并不是每个人都是人际交往的高手,即使是最和蔼的同事也可能因为与你或你的拉取请求完全无关的原因而心情不好。俗话说,“了解一切,一切都可以原谅(Tout comprendre c’est tout pardonner)。”:如果我们能真正了解一个人内心的秘密,我们就不会对他有苛刻的想法。
“我知道你想干什么了”
有些人的代码审查意见虽然算不上粗鲁,但也可能非常居高临下,这几乎和粗鲁一样糟糕。你知道那种话:“嘿,你可能没意识到这一点,但你应该在这里使用指针接收器。我来给你演示一下......”
也许他们是想表示友好,但实际上,我们听到的是:“你根本不知道自己在做什么。幸亏我来了,才没让你出丑”。的确如此。有趣的是,这样做的人通常自己也不太懂,也许这就是为什么他们会这么快就向别人指出他们不懂的地方。
这种情况在科技行业经常发生,你会发现,那些最懂的人往往不会说太多,而那些什么都不懂的人似乎总是滔滔不绝。
也许这是因为,在科技行业,人们往往可以不知道自己的无能。并非所有行业都如此宽容。
例如,在航空领域,那些大大高估自己技术水平的人都死了。-- 菲利普·格林斯潘(Phillip Greenspun),摘自杰西卡·利文斯顿(Jessica Livingston)的《Founders at Work》
吹毛求疵
我们在前面看到,“不是我的风格”的注释并不能为代码带来真正的价值;它们只会制造混乱。每次更新这部分代码时,它也会累积一些浪费的随机修改,以适应审查者的 "风格"。因此,虽然你不会提出这些意见,但当你收到这些意见时该怎么办呢?
我的建议可能看起来有些激进,那就是干脆忽略它们。有人发表评论,并不意味着你必须自动回复。如果你不回复,他们就不太可能跟进。
如果他们跟进了,如果他们继续坚持要修改(假设他们有能力坚持),你可以和他们私下谈谈,澄清他们认为的屏蔽问题到底是什么。
是纯粹的风格问题,还是掩盖了他们没有充分说明的更重要的问题?如果他们有什么不能接受的地方,你有权礼貌地询问他们。
以身作则
在你的职业生涯中(我们希望你的职业生涯漫长而成功),你会发现,了解人和处理人际关系是一项比简历上任何技术缩写都更有价值的技能。协作是一项棘手的工作,唯一比开发软件更难的事情是团队合作开发软件。
当你以善意、体贴和尊重的态度对待周围的人时,这也是最好的方式教会他们代码审查优雅之道:以身作则就相当不错。