如何进行代码评审？

Question

我的前一个问题与如何在开发人员之间推进代码评审有关。在这里，我感兴趣的是如何执行代码评审会话，这样审阅者和审阅者都会对此感到舒服。

我以前做过一些代码审查，经历过非常不愉快的事情。我的前任经理会来找我们--临时的--告诉我们向他解释我们的代码。由于他对代码库不太熟悉，每当他要求我解释我的代码时，我会发现自己花了大量的时间来解释我的代码的最基本的结构。因此，每次审查都会持续太长时间，这一过程将使我们两人精疲力竭。

一旦我解释完我的工作，他就会继续提出问题。他提出的大多数问题本质上都是表面问题(例如，不要对这个代码块使用region，将变量名从xxx更改为yyy，尽管后者更没有意义，等等)。在尝试了几轮这个过程之后，我们发现评审过程并没有为我们两个人带来多大的好处，于是我们停止了。

您将如何使每个代码评审成为一种自然的、令人愉快的思想刺激、错误修复和相互学习的体验？此外，在代码签入之后，您多久进行一次代码评审？你是否每周分配一个固定的时间来做这件事？您在代码评审过程中遵循的指导原则是什么？

最后，动机:我到底想让我的团队退出代码评审吗？答:我希望我的团队成员能够互相负责对方的工作。这是主要动机。第二个动机:我希望所有代码都易于阅读/修改/扩展，这样就可以轻松地添加新特性。

Answer 1

• 编码指南:首先，我将有正式的编码指南和工具进行初步检查(主要根据个人喜好进行讨论)。
• 合格评审者:我认为经理在默认情况下不具备进行代码评审的资格。应该是熟悉代码库并对编码有扎实理解的人。
• 讨论本身就是一个论点:如果你有名字讨论，而这意味着这个名字是错误的，那它就不应该留下讨论的空间。同意一个名字。
• 开放交流:这一点对于文化和人们如何沟通来说是最困难的。代码评审应该是人们一起努力提高代码的质量。它应该是公开讨论，有坚实的论据和坚实的反论点，并达成最终协议。代码审核者必须能够清楚地陈述他的观点，被评审者必须能够清楚地为他的选择辩护，并且两者都必须愿意被证明是错误的。如果您不能完成这项工作，请让其他人来进行回顾或考虑code-演练，尽管如果文化错误，演练可能会更糟。
• 建议与决定:如果沟通比这更粗糙，这可能是一种选择。正式同意审查员可以提出建议，但决定取决于被评审者。这设置了场景，并为双方提供了预期的指导。

希望这能有所帮助。

Answer 2

以下是一些似乎很有效的指导方针。

• 在提交之前或之后，代码检查每个签入。代码审查的最大好处之一是，在bug影响下游的人之前，它可以快速捕获某些类型的bug。此外，代码在您的头脑中越新鲜，您将能够更快地解决所提出的问题。
• 把注意力集中在代码上而不是人身上。这可能很难，但每个人都应该记住，代码评审的目标是提高代码质量，而不是炫耀自己比同事好多少。作为作者，在评审发生之前，要认识到您的代码可能有错误和/或可以改进。这并不会让你成为一个糟糕的程序员。
• 保持较小的评论(200-400LOC)。除此之外，审查员的效率也大大降低。
• 预计将复习300-500 LOC/小时。如果您遵循此准则和前一条准则，代码评审的持续时间不应超过90分钟。
• 让经理们远离代码评审。除非你在一家经理每天都在写代码的公司工作(通常是创业公司)，否则经理不应该参与。让经理参与建立了一个糟糕的社会动态，评审员试图给老板留下深刻印象，而作者工作太辛苦，无法维护自己的代码。
• 用工具。基于工具的代码评审在很多方面都优于会议，但其中的关键(对我而言)是，基于工具的代码评审不需要在日历上占用时间。我的经验是，时间一旦被封锁，它就会被使用。如果会议排定为一小时，则无论会议的目标是否在头10分钟内完成，都需要一个小时(或更长时间)。

其中一些技巧来自SmartBear (我的雇主)在11个同行代码审查的最佳做法上发布的免费白皮书。

Answer 3

我喜欢在大多数开源项目中实践的代码评审形式，这种方式类似于“至少有一个人在登陆之前查看了每个修补程序，补丁的作者必须解决他们建议的改进。”可以称之为“补丁审查”。

凯瑟琳·罗通多在这里有一篇很好的博客文章：http://riarockstars.com/2011/02/10/code-review-an-apple-a-day/

我认为补丁评审主要是一种通信机制。如果你不这么做，那么人们倾向于发明他们自己的编码风格，不知道他们是如何编写代码的，也不知道现有的代码可以重用，等等。此外，如果没有补丁审查，整个项目的架构会变得更加不连贯，因为人们不必与其他人同步。

您可能会认为修补程序评审是“异步对编程”。(与对编程的一个相似之处是，它有助于阻止单个人拥有强大的“代码所有权”。)

其次，修补程序评审往往会捕获相当数量的愚蠢错误，并且您可以强制执行规则，例如“编写单元测试”。

补丁评审中的一个重要因素是补丁必须是可审查的。通常，开发人员把提交修改控制看作是一种任意的快照机制，比如在编辑器中按下保存，或者将正在进行的工作推送到服务器，这样他们就可以从另一个位置到达它。这类补丁是不可审查的。要想被审查，补丁必须是一个连贯的变化，很好的解释，没有“噪音”在其中。显然，代码本身也需要可理解！

git最让人喜爱的地方之一是，它允许您在工作时执行快照/保存方法，然后返回并重新建立基、合并和拆分提交，以便它们成为一个可读的历史，这是有意义的。然后，您可以提交这些补丁供审查。

如果您进行了一段时间的修补程序评审，它会迫使人们编写代码，以便其他人可以读取它，这是非常巨大的。有些开发人员天生无法做到这一点，他们可能不需要在您的团队中。

补丁评审员应该始终拒绝批准对他们来说没有意义的补丁，或者无法阅读或缺少良好提交消息的修补程序。其想法是，修订控制日志和补丁应该讲述一个很好的、连贯的故事，这是有意义的。