没错，Code Review系列还在继续，今天我们一起来聊一聊如何提出好的Code Review反馈。
Code Review是保证代码的质量和可维护性，以及向团队成员分享知识的重要手段。但是，随着团队产出代码质量的提升，Code Review所带来的价值反而会下降。本文我将向你说明如何提出好的Code Review反馈。这一调研结果是来自于对微软数百人的高效工程师的访问。
代码审查检查的是关于代码的问题和质量
在一次代码审查过程中，由一名工程师做出的修改将由其他工程师来进行检查和讨论。代码审查的主要目标是查出代码的问题，保证代码的质量。即使Code Review还带来了一些其他的像传播和学习知识这样的好处，但我们仍要谨记这两个最重要的目标。
有些团队担心的，有些团队已经遇到了代码审查的主要缺点：降低编码速度。这意味着团队的生产效率被代码审查拖慢了。
为什么会这样呢？
前文我们已经有过介绍，降低团队效率的原因可能有很多，但通常是反馈的等待时间长和响应慢有关。如果再加上毫无意义的反馈交流，那么代码审查对于所有开发者都将是噩梦般的存在。但团队可以轻松规避这些问题。
本文我主要向你介绍的是如何提出有价值的反馈。
微软的代码审查研究
在微软，我进行了一项研究来了解代码审查。在这其中一项，我们分析了超过200万条代码审阅批注，以了解哪些反馈是有价值的，哪些是在浪费时间。但我们要先从代码审查应该看什么来介绍。
代码审查时应该看些什么
我们假设你被要求来审查一些代码。代码的作者发给你了几个代码文件，并对代码修改的目的做了简单的描述。那么现在你要开始审查代码了，你应该关注什么？
- 功能缺陷
- 逻辑问题
- 缺少验证（例如边界问题）
- API的用法
- 设计模式
- 架构问题
- 可测性
- 可读性
- 安全问题
- 命名约定
- 团队编码规范
- 文档
- 使用最佳做法
- 特定语言的问题
- 使用过期方法的问题
- 性能（比如复杂度）
- 替代解决方案
真多啊！为了系统的查找这些问题，最好使用代码审查清单，它可以帮助你快速检查这些问题，并确保不会遗漏。我会写一份完整的，更加详细的代码审查清单，记得订阅我，方便第一时间获取。
现在，你看了所有的问题，你一定会问自己：哪些是最有价值的问题？
哪个反馈是最有价值的
让我们来再次想象一下实际工作中你是如何开始一次代码审查的。
也许你打开审查之后，会先浏览所有文件，然后调整自己。哪里发生了变化？代码的哪部分受到影响？修改的中心在哪？
当你已经熟悉了这些修改以后，你就会注意一些问题了：注释和变量中的错别字，缺少注释，缩进等代码风格相关的问题，即使这些是要寻找的问题，也不要陷入这些问题中。实际上，这些问题是有价值的，但并不是我们最主要的目标。
那么，还要看哪些问题呢？
有关缺陷、验证缺失和最佳实践的反馈是最有价值的
最有价值的代码审查反馈都是关于代码中实际问题的。所有开发人员都将这种反馈视为最有用的类型。但我们发现，在研究中这种问题只占全部反馈的很小一部分。下图中，你可以看到代码审查过程中开发人员都在讨论哪些问题，以及对于作者而言他们认为哪些是有价值的。
最有价值的代码审查批注解决了以下问题：
- 功能缺陷。这看起来是轻而易举的，评分最高的代码审查反馈是发现系统中的功能缺陷。但代码审查并不是发现功能缺陷的主要手段，事实上只有一小部分批注是关于功能缺陷的。但是，如果找到一个，那么代码审查带来的收益就是巨大的。
- 验证缺失和极端案例。开发人员认为显示了被遗忘的方案、未涵盖的逻辑问题或极端情况的代码审查反馈是非常有价值的。这种反馈围绕寻找当前方案可能失败的情况和备用方案来展开。
- 最佳实践和代码约定。代码审查对于维护一致的、可维护和可理解的代码库非常有用。所以，那些指出代码中包含不符合代码规范和最佳实践的反馈是很有价值的。
- API使用和设计模式。其他的有价值的反馈主要是关注API或第三方库使用是否正确，或者是缺少或错误的使用了设计模式。
代码审查反馈是一把双刃剑
我们讨论的一些问题并不像功能缺陷那样更容易显示价值。这些问题可能很有价值，但也有可能是在浪费所有人的时间。可能你已经猜到了，我们在讨论的是代码风格、代码规范和注释。这类问题通常被称为“挑剔问题”。
文档、编码风格和编码规范。处理丢失或过时的文档，突出注释中的错别字，或指出不好的命名是你经常收到的代码审查反馈。但它们真的有价值吗？
有时代码审查者并不能马上看到反馈的价值。但是找出错别字也不是大的问题不是吗？这些批准真正的价值是随着时间流逝而带来的复合效应。快速解决此类问题可以保证代码库一直保持可维护性和可理解性。
尽管如此，它们还是会被看作是“挑剔”的行为，并且这个词已经具有负面含义了。所以团队必须保证所有人都能理解这类反馈的价值所在。
另一方面，避免对代码缩进和代码风格进行冗长而重复的讨论是非常重要的。这无疑会拖慢团队的生产效率。为了让团队保持生产力，让我们先制定一种代码风格，然后继续前进！
超出代码审查范围的反馈是无用的
多数被认为是有价值的反馈都是关注当前范围代码审查。但是，代码审查的范围并不是代码更改文件中可见的代码，也不会超出代码修改的目的。因此，提出实施范围之外的问题对于大多数开发人员来说是无用的。
- 替代解决方案。即使替代解决方案被认为是有价值的，但是对当前代码没有明显价值的实现并不能帮助你的团队提升生产效率。
- 现有技术债务和重构。类似的，开始突显的旧的技术债务和潜在的重构机会超出了常规的代码审查范围。这些问题应该单独讨论。
- 计划和未来的工作。另一个没有用的反馈类型就是批注过于关注未来的工作或者不在当前开发周期的工作。如果你看到这些问题，用backlogs或issue tracker这样的工具记录下来，这样做对你的团队是有价值的。之后，在合适的时间可以拿出来讨论。
- 提出问题只是为了了解实现。即使代码审查是一种学习和分享知识的工具，但提出了解实现的问题并不是代码审查的目的。别忘了，代码审查是作者为了获得同意以便继续工作。
如果你不理解代码时应该做什么？
当你不理解代码时应该做什么呢？你怎么才能提出有价值的反馈？
这是一个好问题，实际上，研究和经验告诉我们，如果你不理解代码，你无法提供有价值的反馈，至少能提供的不多。
如果是这种情况，你最好先了解一些潜在问题。你为什么不能理解代码？因为你是团队的新成员？因为你缺乏经验？你以前没有使用过代码库？新编写的代码一团糟？
如果是最后一个原因，那么你所有的问题都是有效的，应该作为代码审查的一部分。但是可能你添加的不仅仅是问题，也许会加一些关于如何改进代码，为什么难以理解代码的反馈等等。
如果你对代码不熟悉怎么办？
如果你之前在工作中没有接触过代码库，你可能很难理解代码审查中的内容。一个好的方法是求助同事，请他/她为你解释一遍代码。这里重要的是不要在代码审查过程中随机询问有关代码库的问题。
没错，学习和传播知识是代码审查的两个重要的好处。不过，这些都是代码审查的附加价值，真正的关注点应该在于检查代码是否正确以及是否高质量。
除非明确告诉你通过这种方式来教你。否则你应该正常做代码审查，这比你只观察别人怎么做要好。慢慢的，你就能更好的理解代码，了解团队惯例和最佳实践，以及向代码审查添加有用的反馈。
缺乏经验的开发人员提出有价值的反馈较少
不只是你，也不是初级开发者的错。这只是一个事实。我们的研究表明经验丰富的开发人员更能提出有价值的反馈。刚开始在组织内部工作的经验较少的开发人员，在前三个月很少能提出有价值的反馈。之后我们可以看到他们提出反馈的价值在一年内如何增加和稳定。
为了提出有价值的反馈，你必须熟悉代码
多项代码审查研究表明，有价值的反馈多数来自于曾经参与开发或审查对应代码的开发人员。好消息是，只要之前改过一次就足够了。也就是说在我们的研究中，一次修改代码的开发人员和修改了上百次代码的开发人员在审核时没有显著的区别。如下图所示。
领域专家可以提高你的代码审查价值
来自其他团队或者是跨团队的领域专家作为审阅者会对你的代码审查更有价值。你可以选择增加安全专家、大数据专家或可用性专家，即使他们并不像你的团队那样熟悉你们的代码。
这样做的好处是给代码审查带来了特别的经验和外部的视角。他们进行代码审查的目的也不同。他们可能不会去寻找最佳实践和团队规范，而是检查代码中你需要他们检查的真正存在的问题。
像对待自己一样对待别人
代码审查是很社会化的活动，在致力于积极打造反馈文化的团队中，它被高度赞赏和高价值的工程实践。不幸的是，并不是任何地方都是这样。在一些团队中，代码审查被滥用为权利展示甚至权利争斗的工具。这样的代码审查没有任何帮助。
如果你想要从代码审查中受益，了解如何提出建设性反馈是非常明智的选择。指出一些代码的质量不够高。如果你批判同事的代码，请务必始终以尊重的方式进行，并一直提出改进建议。
另一方面，也不需要在代码审查过程中加入过多的赞美之词。在微软的代码审查研究中我们发现，作者不太在意对他们代码的称赞。
为什么会这样？我们要再次提到代码审查的目标。通常每个批注都是一个小的工作项。即使是赞美，有太多也不会增加价值。它只会加剧处理批注的工作量。
指出良好的工作对于团队合作精神是必不可少的，并且是一个很好的团队合作的动力。但是这最好是在其他场合提出，比如会议上或者是咖啡时间。
外部情况影响反馈的价值
还有几件事会影响你在代码审查过程中获得的价值。在研究中我们发现开发人员很难查看非代码的文件，比如配置文件或者编译文件。换句话说，开发人员会针对源码提供更有价值的反馈。
影响代码反馈质量的另一个因素是审查文件的数量。需要审查的文件数越多，你收到反馈的质量就越低。保持审核的小巧有很多好处，并且是最有价值的代码审查最佳实践之一。
总而言之，有价值的反馈是针对代码审查目标的反馈：检查当前代码更改是否正确以及是否高质量。不利于实现此目标的讨论应该在代码审查过程之外讨论。因为好的代码审查反馈也是及时提出的反馈，它会帮助作者更快通过审核。
