关于代码审查，那些你不曾关注的细节
作者 | Marcus Eisele
译者 | 弯月
出品 | CSDN（ID：CSDNnews）
在工作中，我们都要进行代码审查。每个人都知道代码审查，每个人都会做代码审查（至少我希望你会做）。但如果花点时间讨论一下，你就会发现在“良好的代码审查应该做些什么”这个问题上，可谓仁者见仁智者见智。每个参与者应尽的义务是什么？代码审查的最佳方式又是什么？
什么是代码审查？
首先让我们来看一看什么是代码审查？维基百科上的代码审查定义如下：
代码审查（有时称为同行审查）是一项确保软件质量的活动，是由一个或多个人通过查看和阅读部分源代码来检查程序的一种方式，代码审查通常发生在实现完成后或实现中间阶段。参与审查的人员中必须至少有一人不是代码的作者。执行审查的人员称之为“审查人”，但不包含作者本人。
——维基百科
这段引文后面还写了代码审查的目标，其中，除了找出被审查代码中的质量这一主要目标外，通过执行这些审查还可以实现：
提高代码质量
保持项目的一致性
发现 bug
学习（通过代码被审查）和教学（通过审查其他人的代码）
营造共同的责任感
通过他方查找代码中的小错误，防止这些小错误日积月累腐蚀代码
寻找更好的解决方案的常见方法
从上述定义中我们能看出什么？代码审查是一种好方法，它可以保持软件的可维护性，并在软件投入生产之前发现 bug。除此之外，该方法还有助于教导团队的新成员，即使没有正式的培训或训练，我们也可以通过代码审查将一些技巧传授给他人。
但如果仅参照该定义展开代码审查的工作，我们仍然不清楚应该做些什么？下文将阐述每个参与者应尽的义务，并从中总结出代码审查的最佳方式。
被审查者视角：不给审查者添麻烦
首先申明一点：代码审查不是为了评判或审查某个人的编程能力。“代码审查只关注代码”，无论这段代码是团队中的高级开发人员写的，还是新人实习生写的——被审查者的职责在审查之前就开始了。
在我看来，被审查的人应该尽量减轻审查者的工作，试着换位思考。我认为加大审查者工作难度的主要因素如下：
将重构的代码和新代码的实现混合在一起
在提交功能变更时，重新格式化整个代码库
提交代码时不写提交注释（我会在后面讨论该话题）
在一次代码审核（或一次代码提交）中提交多个功能
需要考虑的因素
在写完上面这段话后，我想起在代码审查中最烦人的因素是“干扰”和“大小”。所谓干扰，我指的是与提交注释无关的一些变更，它们往往会造成思想负担。因为在审查过程中，你无法确定眼前的这些代码只是外观上的变化还是非常重要的功能。
其次是大小——一次只应该提交一个变更。如果你遇到地问题是提交过大，那么对应地解决方法就是拆分之后再分别审查，然后再将这些分离的提交合并到临时的功能分支上，最后再合并到主分支上。由此，我总结出了以下几条代码审查的最佳方式：
1. 单独提交代码清理命令（重新格式化或修复拼写错误等）和重构
我甚至不建议将重新格式化与重构混合到一起。如果你想重构代码，那么请注意用正确的格式。如果代码中只有与重构相关的变更，那么代码审查会容易得多。当代码中出现大量基本上只是清理命令的变更时，我们有时很容易忽略小的变化（还记得我说过的干扰吗）。
2. 编写相关的提交说明
务必确保你的提交注释可以很好地向审查者说明提交的内容，还有尤其要说明代码变更的原因。如果你的设计受到了很大限制，也一定要写好说明。
如何书写好提交注释？下面是我在工作中提交代码时写的提交注释，这也是我们的团队惯用的格式：
XXX：添加一些新功能（简短的说明）
对于代码实现更为详细的描述
你可以写很多行，但还可以包括：
* 项目符号
* 不同的细节
* 如果你有很多点，一行写不下，那么也可以在各个点之间插入空行
引用与之相关的问题
其中 XXX 是问题的链接（例如 JIRA-007：标题），如果代码变更没有相应的追踪问题，那么可以只是用关键词替代，如 FIX、BUG 或 MAINT（维护）。
上述有关提交说明的建议不仅针对代码审核，更是普遍适用的最佳方式。提交说明中如果漏掉了什么重要的信息总是令人恼火，相反，清晰明了的提交注释也会令人心情愉悦。通常在审核代码遇到问题时，就可以试着看能否从提交注释中找到答案。
3. 只提交准备好审查的代码
这也是对他人的尊重。如果我希望得到另一个人的建议，就不会给他制造不必要的麻烦。因此，请确保你的代码通过了所有测试。另外，在让别人审查你的代码前，先进行自我审查，仔细看看你提交的代码差异。
4. 审查期间不要更改代码
这种做法会给审查者带来更多压力，以致中断审查进度。如果你想修改审查过程中发现的问题，那么请确保在接受审查的代码基础上再另建一份提交。如此一来，审查者就可以在现阶段审查完成后，再来看你新修改的代码。最终，在所有审批都确认后，你可以将所有的提交压缩成一个。
回顾被审查者的工作，我们可以得出一个结论，即不要给审查者制造不必要的麻烦当属代码审查过程中被审查者的最佳行为准则：
确保你的代码通过了自己的审查，并且你没有发现任何明显的问题，可以放心地合并代码（如果你发现了问题，并想讨论某些内容，那么提前跟你的审核者打招呼）；
代码中没有混合不相干的变更，不会太长也不会增加阅读难度；
针对代码变更写好提交注释，明确交待变更的目的
「对码不对人」的审查者
首先再次申明：代码审查不是为了评判或审查某个人的编程能力。
我觉得这点对审查代码变更的人而言更重要，我建议你在评论时更加谨慎，通常可能只是因为你没有看到写代码的人在实现过程中遇到的问题。那么，在代码审查时究竟应该做些什么？通常我会检查几个重点项，下面我将据此总结出代码审查的最佳方式。
目的
我的第一个检查点是代码是否与提交注释相符，如果两者存在差异，那么就很难验证代码的正确性。
实现
在验证了实现的目的后，我会验证实现本身，在读完提交说明后，我问自己的第一个问题就是：“我应该如何实现？”
接下来，我会将审查的代码与我想象的解决方案做比较。如果我的想法与审查的代码之间存在很大差异，那么通常我会再回头查看提交注释，或前一次变更，并快速整理我设想的版本。
通常在几分钟后，我就会知道自己的解决方案是否可行，此时我需要考虑的因素包括：
性能（如果是在同一个代码库的话，通常不那么重要）
可读性（恕我直言，这几乎是最重要的）
尝试覆盖比被审查的解决方案更多的极端情况
是否可以用更好的代码样式（比如更多可重用的代码）来实现相同的功能
然而，实际情况可能往往有所不同——你想到了一个解决方案，同时惊讶于你所审核的代码作者的睿智。如果真是如此，那简直太棒了，因为你不仅学到了新东西，而且你需要做的就只是看看眼前的代码在格式和编程风格上是否与开发指南相符。
可维护性
对我来说，可维护性是最重要的因素之一（这可能有偏见，因为目前我正在开发一个长期项目），但若能不影响将来的实现速度也依旧非常了不起。
出于可维护性的原因，我会直接查看代码测试。如果没有，那么对我而言无疑是个坏消息，因为这通常意味着我“需要动手写一些测试用例”。
如果代码中有测试用例，那么我会验证它们是否在测试正确的东西，还有变更后的 API 的使用方法以及测试用例是否合理。通过单元测试我们就能看出实现的使用方法。如果某个实现难以阅读或难以使用，那么大多数时候只能说明该实现不完美。
接下来我会注意 API 级别的重大变化。如果 REST 控制器被修改了，我就会很担心，因为我不想我们的变更牵扯到其他服务的部署。
理想状态下，这不是问题，因为我们两边会同时部署，但大多数情况下，我会避免这种做法，同时采用一个不会破坏客户端的版本。负责服务间通信的 DTO（数据传输对象，Data Transfer Objects）的变更亦是如此。如果DTO中引入了一个新值，我们就需要格外谨慎。
下一步我会纵观实现整体。如果其很难理解，我会想办法（就像前面想象代替实现方案一样）让它更加容易阅读和理解；如果我想到更好的方法，也会征求代码作者的意见，大多数时候，我们会迅速达成共识或商量出一个折中方案。
最后我需要检查的是外部文档。如果有外部文档（例如外部 wiki 或 README.md），我会快速检查文档是否反映了代码中的变更。
安全
一般来讲，代码都需要拥有某种安全机制（例如 OAUTH 层，或 REST 控制器的 BASIC AUTH）。在添加新代码时，我会验证这些代码是否得到了正确的保护。
写评语的方法
在写审查评语时，一定要很友好，还要让你的评语尽量简洁准确。请确保评语针对的是代码，而不是写代码的人。尽可能避免使用所有格（比如你的、我的等等），这些词语会让人误解，尽管你想说的是代码本身。
你需要写明你的评语是代码变更要求，还是可能需要讨论的意见。如果你在一段非常好的代码中写了很多挑剔的评语，那么一定要给写代码的人一些赞美的话。
最后，总结代码审查中，审查者的最佳操作方式：
保持友好
审查代码，而不是写代码的人
评语要简洁准确
在最后不要忘了为好的代码点赞
表明你是否需要等代码修改完毕后再看一次（如果你们的审查工具不支持这一功能的话）
希望这篇文章可以帮助你了解“代码审查的最佳方式”。每天在工作中，我都会遵照这些准则，这对我和我的团队都有很大的帮助。如果你对该主题有其他看法，也请告诉我。
原文：https://programmerfriend.com/index.php/2018/12/10/code-review-best-practices/
本文为 CSDN 翻译，如需转载，请注明来源出处。
推荐阅读：
点击“阅读原文”，打开 APP 阅读更顺畅。