如何让你的代码评审员爱上你
原文由 Michael Lynch 于 发布，订阅该博客
谈到代码评审，人们往往把焦点放在评审者身上。但编写代码的开发者，与阅读代码的人一样，对评审同样重要。关于如何为评审准备代码，几乎没有任何指导，因此作者们常常因为无知而把这个环节搞砸。
本文将介绍作为作者参与代码评审的最佳实践。事实上，读完这篇文章后，你在提交代码供人评审方面的水平会高到 让你的评审员真的爱上你。
但我不想让评审员爱上我
他们就是会爱上你的，接受现实吧。从来没有人在临终时抱怨爱上自己的人太多了。
为什么要改进代码评审？
改进代码评审的技巧对你的评审员、你的团队，以及最重要的——你自己，都有好处。
学得更快：如果你恰当地准备好变更列表，就能把评审员的注意力引导到有助于你成长的地方，而不是枯燥的格式问题上。当你表现出对建设性批评的重视时，评审员也会给出更好的反馈。
让他人变得更好：你在代码评审中的做法会为同事树立榜样。高效的作者实践会潜移默化地影响队友，而当他们把代码发给你评审时，你的工作也会更轻松。
减少团队冲突：代码评审是引发摩擦的常见源头。以审慎、认真的态度对待评审，能最大程度地减少争执。
黄金法则：珍惜评审员的时间
这条建议听起来显而易见，但我经常看到作者把评审员当成私人质检员。这些作者完全不花力气去发现自己的错误，也不去为可评审性而设计变更列表。
你的同事每天来到公司，专注力都是有限的。如果他们把其中一部分分给了你，那就意味着他们无法用在自己的工作上。让他们的时间发挥最大价值，才是公平的做法。
当双方互相信任时，评审效果会大幅提升。当评审员相信你会认真对待他们的反馈时，他们会投入更多精力。如果你把评审员视为必须克服的障碍，能从他们那里获得的价值就会大打折扣。
技巧一览
- 先自己评审一遍代码
- 写出清晰的变更列表说明
- 把简单的事自动化
- 用代码本身回答问题
- 严格控制变更范围
- 将功能性与非功能性变更分开
- 拆分大型变更列表
- 优雅地回应批评
- 当评审员出错时保持耐心
- 明确地传达你的回应
- 巧妙地索取缺失的信息
- 平局时永远让评审员获胜
- 尽量缩短评审轮次之间的等待时间
1. 先自己评审一遍代码
在把代码发给同事之前，先自己读一遍。不要只检查错误——要想象自己是第一次读这段代码。有哪些地方可能会让人困惑？
我发现在写完代码和评审之间稍作间隔很有帮助。人们常常在一天结束时匆匆提交变更，但那正是最容易忽略低级错误的时候。不如等到第二天早上，用全新的眼光再看一遍变更列表，然后再交给同事。
尽可能采用评审员的视角。使用他们将看到的同一个 diff 视图。在 diff 视图中比在常规的源码编辑器里更容易发现愚蠢的错误。
别指望自己能做到完美。难免会发出带有忘记删除的调试代码或本想排除的无关文件的变更列表。这些错误并非世界末日，但值得留意。关注自己容易犯错的模式，并思考如何建立机制来避免。如果这类错误过于频繁，就等于在告诉评审员你并不珍惜他们的时间。
2. 写出清晰的变更列表说明
在上一份工作中，我作为开发者导师计划的一员，定期与一位资深工程师会面。第一次会面前，他让我带一份自己写过的设计文档。当我把文档递给他并解释项目的背景以及它如何契合团队目标时，他皱起了眉头。“你刚才说的这些，都应该写在设计文档的第一页上，”他直言不讳地说。
他说得对。我写设计文档时只设想了队友会怎么读，却没有考虑到其他读者。除了身边的队友，还有合作团队、导师以及晋升委员会等更广泛的受众。他们都应该能够理解这份文档。从那次谈话之后，我总会思考如何为自己的工作补充背景，让人一看就明白。
你的变更列表说明应该概括读者所需的全部背景知识。你写说明时心里可能想着某个特定的评审员，但他们未必拥有你想象的上下文。此外，你的其他队友也可能需要阅读这个变更列表，而未来查阅变更历史的人也应该能明白你当初的意图。
一份好的变更列表说明会从宏观层面解释这次变更实现了 什么，以及你 为什么 要做这次变更。
想更深入地了解如何写出优秀的变更列表说明，请参阅我的另一篇文章——《如何写出有用的提交信息》。
3. 把简单的事自动化
如果你指望评审员来告诉你大括号放错了行，或是你的改动破坏了自动化测试套件，那你就是在浪费他们的时间。
自动化测试应该是团队标准工作流程的一部分。评审应该在持续集成环境中所有自动化检查都已通过之后才开始。
如果你的团队执迷不悟，拒绝投入持续集成，那就自己把这些检查自动化。在开发环境中加入 git pre-commit 钩子、代码检查工具和格式化工具，确保每次提交时代码都符合规范并保持预期行为。
4. 用代码本身回答问题
这张图有什么问题？
作者帮我理解了这个函数，那下一个读到它的人怎么办？难道要他们去翻变更历史，把每一次代码评审讨论都读一遍？更糟的是，作者直接走到我工位前当面解释，这既打断了我的专注，也保证了其他人永远无法获取这些信息。
当评审员对代码的工作方式感到困惑时，解决办法不是只向这一个人解释。你需要向所有人解释清楚。
回答他人疑问的最佳方式是重构代码，消除困惑。你能否通过重命名或重构逻辑让代码更清晰？代码注释是可以接受的方案，但远不如能自我说明的代码。
5. 严格控制变更范围
范围蔓延是代码评审中常见的反模式。开发者本来在修复一个逻辑错误，却在过程中注意到一个界面瑕疵。“既然都到这儿了，”他们想，“就顺手把另一个问题也修了吧。”但这样就把事情搞混了。评审员不得不去分辨哪些改动是为了目标 A，哪些是为了目标 B。
最好的变更列表只做一件事。变更越小、越简单，评审员就越容易在脑中掌握全部上下文。将不相关的变更解耦，还能让你把评审并行分发给不同的同事，从而缩短变更的周转时间。
6. 将功能性与非功能性变更分开
最小化范围的推论，就是要把功能性变更和非功能性变更分开。
不熟悉代码评审的开发者经常违反这条规则。他们只改了两行代码，结果编辑器自动把整个文件重新格式化了。开发者要么没意识到发生了什么，要么觉得新格式更好，于是就把两行功能性改动淹没在数百行无意义的空白改动中发了出去。
你能在这份被空白噪音淹没的变更列表中找到那处功能性改动吗？
杂乱的变更列表是对评审员的极大不尊重。纯空白改动很容易评审。两行的改动也很容易评审。但淹没在空白改动海洋中的两行功能性改动，既繁琐又让人抓狂。
开发者在重构时也常常不恰当地混入其他改动。我很喜欢队友重构代码，但讨厌他们在重构的同时改变代码行为。
这份变更列表只改动了一处行为，但重构带来的改动把它掩盖了。
如果一段代码既需要重构又需要改变行为，应该分成两到三个变更列表来做：
- 添加用于验证现有行为的测试（如果还没有的话）。
- 在保持测试代码不变的前提下重构生产代码。
- 修改生产代码中的行为，并相应地更新测试。
在第 2 步中保持自动化测试不变，你就向评审员证明了重构没有改变行为。到了第 3 步，评审员就不必再去分辨行为改动和重构改动，因为你已经提前将它们解耦了。
7. 拆分大型变更列表
过大的变更列表可以说是范围蔓延的丑陋近亲。假设开发者为了引入功能 X，必须修改现有库 A 和 B 的语义。如果改动量很小，那还好；但太多这种大范围的修改会让变更列表变得无比庞大。
变更列表的复杂度会随着所触及代码行数的增加而呈指数级增长。当我的改动超过 400 行生产代码时，我就会在请求评审前寻找拆分的机会。
与其一次性改完所有东西，能否先修改依赖，再在后续的变更列表中添加新功能？能否先加入一半功能仍保持代码库处于正常状态，另一半放到下一个变更列表中？
把代码拆开、找到一个能独立运行且易于理解的子集是件繁琐的事，但这样能获得更好的反馈，也能减轻评审员的负担。
8. 优雅地回应批评
毁掉一次代码评审最快的方式，就是把反馈当成针对个人的攻击。这很有挑战性，因为许多开发者以自己的工作为荣，并将其视为自我的延伸。如果评审员措辞不当，把反馈说得像人身攻击，那就更难应对了。
作为作者，你终究能控制自己对反馈的反应。要把评审员的意见当作对代码的客观讨论，而不是对你个人价值的评判。防御性的回应只会让情况更糟。
我会试着把所有意见都当作有益的教训。当评审员发现我代码中一个令人尴尬的错误时，我的第一反应是找借口。但我会克制住自己，转而称赞评审员的细致入微。
当评审员发现你代码中隐蔽的错误时，要表达感谢。
令人意外的是，当评审员能发现你代码中细微的缺陷时，这反而是个好兆头。这说明你把变更列表组织得很好。没有了格式糟糕、命名混乱等显而易见的问题，评审员就能深入关注逻辑和设计，从而给出更有价值的反馈。
9. 当评审员出错时保持耐心
评审员有时也会完全弄错。就像你会不小心写出有缺陷的代码一样，评审员也可能误解原本正确的代码。
许多开发者对评审员的错误会产生防御心理。他们觉得有人用根本不成立的批评来指责自己的代码，是一种冒犯。
即使评审员弄错了，这仍然是一个警示信号。如果他们会误读，其他人会不会也犯同样的错误？读者是否需要付出异乎寻常的仔细程度，才能确信某个错误其实并不存在？
当评审员犯错时，要抵制住证明他们错了的冲动。
尝试通过重构代码或添加注释，让代码的正确性更加显而易见。如果困惑源于晦涩的语言特性，就用非专家也能理解的方式重写代码。
10. 明确地传达你的回应
我经常遇到这样的情况：我给别人提了意见，他们更新了代码以回应其中的部分反馈，却没有写任何回复。现在就陷入了含糊不清的状态：他们是漏看了我其他的意见，还是仍在处理中？如果我开始新一轮评审，很可能会把时间浪费在尚未完成的变更列表上。如果我选择等待，又可能造成僵局——我们俩都在等对方先行动。
要在团队中建立约定，让任何时候都清楚是谁“手握接力棒”。要么是作者正在修改，要么是评审员正在写反馈。绝不应该出现因为没人知道该谁行动而导致流程停滞的情况。通过在变更列表层面留言来表明何时交接控制权，就能轻松做到这一点。
在变更列表中留言，明确告知何时将主动权交还给评审员。
对于每一条需要处理的意见，都要明确回复以确认你已处理。有些代码评审工具允许你将评论标记为已解决。如果没有这类功能，就遵循一个简单的约定，比如对每条意见回复“已完成”。如果你不同意某条意见，也要礼貌地解释为什么没有采纳。
像 Reviewable 和 Gerrit 这样的代码评审工具提供了让作者将特定意见标记为已解决的机制。
根据评审员付出的努力来调整你的回应。如果他们写了详细的意见来帮助你学习新知识，不要只是标记为已完成。要认真回应，以表达对他们付出的感激。
11. 巧妙地索取缺失的信息
有时代码评审意见留下了太多解读空间。当你收到“这个函数令人困惑”这样的评论时，你可能会疑惑“困惑”到底指什么。是函数太长？名字不清楚？还是需要更多文档？
很长一段时间里，我都苦于如何在不显得防御性的前提下澄清含糊的意见。我的本能反应是问“它哪里让人困惑了？”但这听起来很不耐烦。
有一次，我无意中给同事发了一条含糊的意见，而他的回应方式让我觉得极具化解力：
怎么做会更有帮助呢？
我喜欢这个回应，因为它传达出不设防、乐于接受批评的态度。每当评审员给出含糊的反馈时，我总会用“怎么做会更有帮助呢？”的各种变体来回应。
另一个有用的技巧是猜测评审员的意图，并基于这个假设主动修改代码。对于“这段让人困惑”这样的意见，再仔细审视一下自己的代码。通常总有办法可以提高清晰度。即使修改并非评审员心中所想，主动修改也能向评审员传达你愿意做出改变的态度。
12. 平局时永远让评审员获胜
在网球中，当你不确定对手的发球是否出界时，你会给对方有利的判定。代码评审也应该有类似的预期。
美国网球协会要求球员在做界内界外判罚时，要给对手有利的判定。
关于代码的某些决定纯属个人品味问题。如果评审员认为你那个 8 行的函数拆成两个 5 行的函数会更好，你们双方都没有客观上的“正确”。哪个版本更好，只是见仁见智。
当评审员提出建议，而你们双方支持各自立场的证据大致相当时，就听从评审员的意见。在你们两人之间，他们更能体会初次阅读这段代码是什么感受。
13. 尽量缩短评审轮次之间的等待时间
几个月前，一位用户向我维护的一个开源项目提交了一个小改动。我在几小时内就给出了反馈，但他们随即消失了。几天后我再次查看，仍然没有回应。
六周后，这位神秘的开发者再次出现并提交了修改。虽然我感激他们的付出，但评审轮次之间的延迟让我的工作量翻了一倍。我不仅要重读他们的代码，还得重读自己的反馈来恢复对讨论的记忆。如果他们能在一两天内跟进，我就不必做这些额外的工作了。
六周的停滞算是极端情况，但在同事之间，我也经常看到长时间且不必要的延迟。有人发出变更列表请求评审，收到反馈后，却因为被其他任务分心而将其搁置一周。
除了恢复上下文所浪费的时间，未完成的变更列表还会增加复杂性。它们让每个人都更难追踪哪些已经合并、哪些仍在进行中。部分完成的变更列表越多，合并冲突就越多，而没人喜欢去解决这些冲突。
一旦你发出了代码，推动评审尽快完成就应该成为你的最高优先级。你这边的延迟会浪费评审员的时间，也会增加整个团队的复杂性。
结论
在准备下一个待评审的变更列表时，请考虑那些你能掌控的因素，并利用它们来有效地引导评审。在参与评审的过程中，留意那些会阻碍进展或浪费精力的模式。
记住黄金法则：珍惜评审员的时间。当你让评审员专注于代码中有趣的部分时，他们才能给出高质量的反馈。如果你让他们去费力理清你的代码或纠正低级错误，你们双方都会受到影响。
最后，要用心沟通。简单的误解或欠考虑的评论很容易让评审偏离正轨。在评价他人工作时，情绪很容易激动，因此要警惕那些可能让评审员感到被攻击或不被尊重的陷阱。
恭喜！如果你已经读到这里，你现在已经是一名优秀的被评审者了。你的评审员很可能已经爱上你了，所以要好好珍惜他们。
延伸阅读
- 像人类一样做代码评审：在学习了作为作者的有效实践之后，再来学习如何在作为评审员时改进代码评审。
插图由 Loraine Yow 绘制。由 Samantha Mason 编辑。
评论
登录后参与讨论