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