如何像一个人一样做代码审查（第一部分）
最近，我阅读了一些关于代码审查最佳实践的文章。我发现这些文章把重点几乎全部放在了“找漏洞”上，而几乎忽视了审查工作的其他方面。发现问题后如何以建设性和专业的方式进行沟通？对不起，这一部分似乎完全不重要！只要发现所有的漏洞，剩下的事情自然就会迎刃而解。
于是我有了一个启示：如果这种思路在代码审查中行得通，那么在浪漫关系中是不是也一样？基于这个想法，我准备推出一本新的电子书，用于帮助开发者在他们的感情生活中获得助力：
我这本革命性的电子书会教你如何用行之有效的技巧来最大化发现你伴侣身上的“缺点”。不过，这本电子书并不涵盖以下内容：
- 如何带着同理心和理解，与伴侣沟通你所发现的问题。
- 如何帮助你的伴侣改进他们的弱点。
根据我对现有代码审查文章的阅读，上述这些关于人际关系的部分似乎是显而易见且不值得讨论的。
这样的一本电子书听起来不错吗？我猜你的反应大概是“不要不要不要！”
那么，为什么在谈及代码审查时，我们就只关注“找BUG”了呢？
我只能推测，我读到的这些文章来自未来。那时所有的开发者都是机器人。对于那些不加思考、言辞尖锐的代码批评，他们心怀感激，因为处理这些信息可以温暖他们冰冷的机器人心脏。
但我大胆地假设，你仍旧希望在当下提升代码审查的质量，并且你的同事们都是普通的人类。我更大胆的假设是，你希望和同事之间保持积极的关系，这本身就是一个目的，而不仅仅是为了把“单个BUG的发现成本”降到最低。那么，在这种情况下，你会如何改变你的审查方式？
在这篇文章中，我会介绍一些技巧，把代码审查不仅仅当作一种技术流程，而是同时重视它所具备的社交属性。
什么是代码审查？
“代码审查”这个术语可以指代许多不同形式的活动，从单纯在同事肩膀后面扫几眼代码，到召开20个人的大型会议挨行检查，都算是某种程度的审查。我在这里所说的代码审查，指的是正式且书面的流程，但并没有繁琐到需要一连串线下会议来检视代码的程度。
在一次代码审查中有两个主要参与者：作者（author）——也就是编写代码并将其提交审查的人，以及审查者（reviewer）——负责阅读代码并决定代码何时可以合并到团队代码库中的人。一份审查也可能有多个审查者，但为了简化讨论，这里假设只有你一个人负责审查。
在代码审查开始之前，作者需要创建一个变更集（changelist），即作者想要合并到团队代码库中的一系列源码变更。
审查从作者将变更集发送给审查者开始。审查会分为一个个轮次（round）进行。每一轮包含一次作者的提交和一次审查者针对该提交的书面反馈。每个代码审查都会有一个或多个轮次。
当审查者批准了这些变更后，审查才算结束。通常会用“LGTM”（“looks good to me”）来表示已经通过。
为什么这很难？
如果一名程序员向你提交了一份他们自认为很棒的变更集，而你给出了一长串理由来说明它哪里不好，想要表达好这个信息本身就很棘手。
“这就是我不想念叨IT工作的原因，因为程序员往往很不讨人喜欢……比如在航空领域，如果有人极度高估自己的技能水平，那他很可能早就‘出事’了。”——Philip Greenspun，ArsDigita 联合创始人，节选自Founders at Work 作者很容易把对他代码的批评，视为对他个人专业能力的否定。实际上，代码审查是共享知识和达成更佳工程决策的机会。但如果作者把这当作人身攻击，那一切就无从谈起了。
更难的地方在于，你需要以书面形式表达你的想法，这其实很容易造成误解。作者无法从文字中听出你的语气，也看不到你的肢体语言，因此你要更仔细地组织语言。对于一个本就带着戒备心理的作者而言，哪怕是“你忘了关闭文件句柄”这样无害的提示，都可能被理解成“我真不敢相信你竟然忘了关闭文件句柄！你这个蠢货。”
技巧
- 让计算机处理那些枯燥的部分
- 用风格指南来解决风格争议
- 尽快开始审查
- 先关注高层次问题，再逐步深入
- 多给示例代码
- 不要使用“你”这个词
- 把反馈表述为请求，而非命令
- 把意见与原则挂钩，而不是个人看法
让计算机处理那些枯燥的部分
在各种干扰（比如会议、邮件等）之中，你能专注阅读代码的时间本就有限。你能投入的精神也更加有限。要仔细阅读同事提交的代码，需要高度专注，是一项消耗脑力的工作。不要把这些宝贵的时间和精力浪费在一些完全可以让机器做、并且机器做得更好的事情上。
空白字符（缩进、空格等）错误就是典型例子。对比一下由人力来查找和更正缩进错误，再与自动格式化工具相比，孰优孰劣？
由审查者人工发现并解决
使用自动格式化工具
审查者在代码里搜索空白字符问题并找到一个缩进错误。
审查者写下评论指出这个错误。
审查者再三检查评论，保证措辞清晰，没有责备倾向。
作者阅读评论。
作者修正代码缩进。
审查者再次验证，确保问题已解决。
什么都不需要！
右边之所以是空白的，是因为作者使用了一个能在每次保存时自动处理空白字符的代码编辑器。最坏的情况下，如果作者提交的代码存在空白字符问题，持续集成（continuous integration）会报错，然后作者再去修复，不需要审查者操心。
在代码审查中，找出可以自动化处理的机械性任务，让计算机来做掉。常见的自动化解决方案如下表：
任务
自动化方案
确保代码能正常构建
使用持续集成工具，如Travis或CircleCI。
确保自动化测试全部通过
同上，利用持续集成（Travis 或 CircleCI）。
确保空白字符风格符合团队规范
代码格式化工具，如ClangFormat（C/C++）或gofmt（Go）。
找到未使用的 import 或变量
代码静态检查工具（linter），如pyflakes（Python）或JSLint（JavaScript）。
通过自动化，你能把精力更多地放在有意义的点上——比如功能性错误，或者可读性问题。你不再需要关心 import 的顺序、文件命名规范这些细枝末节。
对作者来说，这同样也是好事。自动化工具能让他们在几秒内就发现马虎错误，而不必等上几个小时。实时反馈也让他们更容易学会如何避免这些错误，改起来也便宜，因为当时的上下文还在脑子里。而且，如果必须得让人来指出愚蠢的错误，宁可让机器来当“坏人”，也不想从你口中听到。
你应该和团队一起，把这些自动化检查直接嵌入代码审查流程（比如在 Git 中使用pre-commit hooks，或者在 GitHub 中使用webhooks）。如果审查流程要求作者手动去运行这些工具，就会失去大部分好处。因为作者偶尔还是会忘了跑这些检查，结果还是得由审查者来关注这些原本该自动化处理的问题。
用风格指南来解决风格争议
对于审查来说，所有跟“编码风格”有关的争论都很浪费时间。保持风格一致当然很重要，但审查现场并不是你和作者较劲大括号该放哪儿的地方。最好的做法是事先准备一份风格指南（style guide），把这些争议隔离在审查流程之外。
一份好的风格指南不仅涵盖变量命名、空白字符等表层内容，还应该说明针对所用编程语言的使用规范。比如 JavaScript 和 Perl 功能非常丰富，实现同一个逻辑可以有多种方式。风格指南应该定义 “统一做法”，以免团队里有人用一套语言特性，另一些人又用完全不同的特性，最终代码变得混乱。
有了风格指南，你就没必要在审查时和作者争论“我的命名规范是不是比你的更好”。直接拿出指南来，根据它决定，然后继续审查其他内容。如果风格指南里没有明确某个问题，那通常也就不值得浪费审查时间去争论。要是你真的遇到一个指南里没有、但又很重要的风格问题，那可以和团队一起讨论，做出决定后写进风格指南里，这样以后就再也不用讨论了。
方案1：采用现成的风格指南 网上能搜到一些已经公开的风格指南，比如Google 的风格指南就很常见。如果它满足你的需求，你就能直接拿来用，省去了从零开始编写的成本。
缺点是，不同组织会根据自身需要来优化风格指南。比如 Google 的风格指南对于新语言特性的使用就比较保守，这是因为他们有庞大的代码库，需要兼容从路由器到最新iPhone在内的各种环境。如果你只是四个人的初创团队，只有一个产品，可能会倾向于更积极地采用新特性或扩展。
方案2：增量式地编写自己的风格指南 如果不想直接采用别人的指南，可以自己编写。每次在代码审查中出现“风格上的争议”，就把问题提出来让整个团队讨论，最终形成一致意见后，写进自己的风格指南。
我一般会把团队的风格指南放在源代码的版本管理中（比如GitHub Pages），并且以 Markdown 的形式维护。这样所有对风格指南的改动也能走标准的审查流程：需要有人明确批准，团队里每个人也都有机会提出不同意见。用 Wiki 或者 Google Docs 也可以，只要保证整个团队能方便地追踪和查看。
方案3：混合模式 你也可以同时结合方案1和2，先采用一个现有的风格指南，再用一个本地维护的风格指南对其进行扩展或覆盖。一个很好的例子是Chromium 的 C++ 风格指南，它基于Google 的 C++ 风格指南，但也在此基础上做了自己的改动和补充。
尽快开始审查
把代码审查当作高优先级的任务。在实际阅读并给出反馈时，可以慢慢来、仔细看，但要尽快开始阅读，理想情况下几分钟之内就能启动。
如果同事向你提交了一份变更集，往往意味着他们在等你的审查才能进行下一步工作。理论上说，作者可以在自己的分支继续开发，然后把审查的改动合并到新的分支里。但实际上，只有少数几位开发者可以高效地管理如此复杂的分支合并，大多数人会在三方差异合并（three-way diff）里耗费太多时间，导致这种分支策略得不偿失。
当你迅速开始审查时，你会形成一个良性循环：你的审查速度只取决于变更集的大小和复杂度，这会激励作者尽量提交小而精的变更集。这样的变更集也更容易、更愉快地审查，你审查得也快，双方都获益。
想象你的同事实现了一个新特性，需要改动1000行代码。如果他知道你能在2小时内审完200行代码，那他就会把这个新特性拆成若干个200行左右的变更集，可能一两天就能全部合并。可如果无论变更集多大你都要花一天时间审查，那么他需要一周才能把这个新特性合并。为了不浪费一周的时间，他就会一次性提交500-600行的变更集。这样一来，你的审查负担更重，而且越大的变更集越难把控上下文，反馈质量也会更差。
无论如何，一轮审查最多不要超过一个工作日。如果你在处理更高优先级的事务，无法在一天内完成审查，就应该告诉提交者，让他有机会把审查重新指派给别人。如果你发现自己经常（比如一个月超过一次）因为忙不过来而拒绝审查，那么很可能意味着你们团队需要减缓开发节奏，以保证能维持合理的审查流程。
先关注高层次问题，再逐步深入
在一次审查轮次中，评论数量写得越多，越有可能让作者感觉压力很大。具体的“警戒线”因人而异，但通常20到50条评论就有“淹没”作者的风险。
如果你担心一次性给太多意见会让作者招架不住，可以先关注高层次的问题，然后在后续轮次里再谈细节。比如先讨论类接口如何重新设计、如何拆分函数等“大结构”上的问题，等这些敲定了，再来谈变量命名、注释是否清晰等“小细节”上的事情。
高层次的改动往往会让你先前关注的那些细节“自动过期”，因为作者在大改时可能把一些低级问题一并解决了。这样，你就不用浪费时间在第一轮就写一堆很具体的细节性评论，作者也不用再处理那些可能不再适用的低级意见。与此同时，这种逐层深入的方法也能帮助双方更有条理地把握变更集的内容。
多给示例代码
在理想的世界里，代码作者对于得到审查意见应该感激不尽。因为审查能让他们学习，也能防止错误出现。但现实中，有很多客观因素会导致作者心情不佳，对审查意见充满抵触。也许他们赶着某个死线，所以除非你立刻盖章通过，否则他们都会觉得你在故意刁难。又或者你们彼此并不熟悉，作者也无法确定你的意见是不是善意的。
要让作者对审查心怀感激，一个好办法就是在审查中“送礼物”。对开发者来说，什么礼物最实用？代码示例。
如果你能写出一些他们需要修改的代码示例，相当于帮他们分担了一部分工作，也能体现你对他们的慷慨和支持。
比如，你发现一个同事还不熟悉 Python 里的列表推导式（list comprehension），而他提交了一段这样的代码：
urls = []
for path in paths:
url = 'https://'
url += domain
url += path
urls.append(url)
如果你只写“能不能用列表推导式简化一下？”就会让他感觉头疼，因为他得花二十分钟去查什么是列表推导式。
相反，如果你直接给出一个示例：
可以考虑用列表推导式来简化，例如：urls = ['https://'+ domain + pathforpathinpaths] 那么，他就会轻松很多。
这种方法并不仅限于一行代码的例子。有时我会在本地新建一个分支，示范如何拆分一个复杂函数，或者给某个边界情况写单元测试等。
需要注意的是，这种“送礼”要用在那些清晰且大多数人都认可的改进上，比如前面那个例子里，减少83%的行数，几乎没有谁会反对。但如果你写了很长的示例，只是为了证明一个“我更喜欢的写法”，而无关功能或性能提升，那么示例代码会让你显得强势而不是慷慨。
另外，最好每轮审查的示例代码不要超过两三个。如果你几乎把整份变更集都替作者写了，就会让对方觉得你不相信他们能写好自己的代码。
不要使用“你”这个词
这一点听起来很奇怪，但请相信：在代码审查里，尽量不要用“你”这个词（英文中的“you”）。
在审查过程中，我们希望决策是基于让代码变得更好，而不是谁提出的想法。作者可能对自己提交的变更集花了很多时间，觉得很有成就感。当他们遇到批评时，下意识就会产生防御心态。
在反馈中避免使用“你”，可以尽量降低作者的戒备心，让他们意识到我们批评的是“代码”，而不是“写这段代码的人”。当作者在评论中看到“你”，往往会把注意力转移到自己身上，而不是那段代码。这样就增加了把批评视为人身攻击的风险。
设想下面这条看起来很中性的评论：
你把“successfully”这个单词拼错了。 作者有可能会有两种截然不同的理解：
- 好心提醒：“你”拼错了“successfully”，但我并没有质疑你的水平，这可能只是个小疏忽。
- 人身攻击：“你竟然拼错了‘successfully’，蠢到家了吧？”
对比一下，如果我们去掉“你”：
sucessfully -> successfully 这样就是一个很简单的“纠错”建议，而不涉及对作者的评价。
幸运的是，只要稍作修改，就能把评论写得既礼貌又不带“你”。
方法1：把“你”改成“我们”
你能不能把这个变量名改得更有意义一点，比如seconds_remaining？ 改成：
我们能不能把这个变量名改得更有意义一点，比如seconds_remaining？ 用“我们”来强调团队对这份代码的共同责任。作者离职了也好，你走了也好，这段代码还在团队里总要有人维护。也许听起来有点怪，好像是我们一起搬沙发，但比起指责口气肯定好很多。
方法2：省略主语
另一种方式是用一种不带主语的简短表达，比如：
建议把变量名改成更有意义的，比如seconds_remaining。 或者使用被动语态也是可行的：
这个变量应被重命名为更有意义的名字，比如seconds_remaining。 你也可以用疑问句的方式，如“what about…”或者“how about…”：
What about给这个变量改成更有意义的名字，比如seconds_remaining？ （在中文里可以类比“是不是可以考虑……”“要不要改成……”等说法。）
把反馈表述为请求，而非命令
代码审查需要更多的谨慎和技巧，因为一不小心就会让人觉得是在对人不对事。按理说，审查时更需要礼貌，但奇怪的是，很多人会在审查评论里直接发号施令，这在日常工作中其实并不常见。比如，很少有人会对同事说“把订书机给我，然后给我拿瓶可乐”，但在审查中却常见“把这个类移到单独的文件里”这种命令式说法。
在审查中，尽量使用显得“过于礼貌”的沟通方式也没什么坏处。把意见写成请求或建议，而不是命令。
对比一下以下两种表述方式：
命令式
请求式
把Foo类挪到单独的文件去。
能不能把Foo类挪到单独的文件里？
大多数人都喜欢对自己的工作有自主权。当你用请求的方式来提出修改意见，作者会觉得自己被尊重了。
同时，这种请求式的表达也让作者更容易做出合理的反驳。也许他们这样做是有原因的。如果你用了命令句，作者的反驳就像是在“顶撞”你；但如果你是请求的口气，他们就可以直接向你解释原因。
看下面两种假设场景，审查者和作者之间的对话感受完全不一样：
命令式（对立冲突）
请求式（合作交流）
审查者：把Foo类挪到单独的文件里。作者：我不想这样做，因为那样就和Bar类分开了。大多数情况下，这两个类会配合使用。
审查者：能不能把Foo类挪到单独的文件里？作者：当然可以，不过那样会和Bar类分开。一般情况下这两个类都是一起用的。你怎么看？
显而易见，当我们用请求的方式提问时，对话就变得更平和，也更容易达成共识。
把意见与原则挂钩，而不是个人看法
当你给作者提出修改建议时，尽量同时说明建议的改动和背后的原因。不要只是说“应该把这个类拆成两个”，可以解释说“现在这个类同时负责下载和解析文件。根据单一职责原则，我们最好拆分成一个下载器类和一个解析器类。”
这样就能让讨论更具建设性。因为你给出的理由是：“我们想要让代码更符合某项原则”，作者想反驳就不能只说“我比较喜欢我现在的写法”。或者说，如果作者只回复“我喜欢我现在的写法”，那就显得没什么说服力。
软件开发既是一门科学，也是一门艺术。有些时候你确实难以用确凿的“原则”去解释为什么这段代码让你觉得怪怪的。可能它就是不美观、不直观，但你说不出具体原因。在这种情况下，尽量保持客观。例如说“我觉得这段逻辑有点难读”至少还能算是客观描述——它说的是你的真实感受，而不是“这个实现就是一团糟”，那就变成一个价值判断，而且可能并非所有人都同意。
如果有条件，提供一些证据链接就更好了，比如指向你们团队风格指南的某个具体条目，或者语言/库的官方文档。高票的StackOverflow答案也勉强可用，但权威性就差了一些。无论如何，最好用一些客观依据来支撑你的意见，而不是纯粹的口头之争。
（完）