13条实践助你更好地完成代码评审
本文要点
- 组织你的提交列表，将它重点突出，并且尽可能小，但是不要推迟测试而使它变得不可靠。
- 只有在有合理的理由时，才提出评论，或者拒绝已经收到的评论。创造一种基于论据、没有抱怨和责备的文化。
- 将评审任务优先安排在编码之前，并保证评审人员，以此来尊重和帮助评审过程。
- 对于棘手和有挑战性的问题，可以考虑口头交流并与评审人员结对。使用工具来检测常规和简单的问题。
- 评审一切。作为一名评审人员，身为开发人员要考虑到一切都在你的评审范围之内。
从小的提交列表，到有效的评论，再到优先考虑评审而不是编码
多年来，我一直在接收代码评审会议的邀请，同时也在发出邀请。如果做得不适当，代码评审可能会令人恼火、花费大量时间，并且对代码质量几乎没有影响。但是，如果做得适当，它可以提高代码质量并减少交付特性所花费的时间。它还有助于发现棘手的bug，加速团队中的知识共享。在代码评审期间发生的对话也可以使我们对问题或替代解决方案有新的见解。
我将baker 的13条最佳实践放在了一起，你和你的团队可以使用它们来提高代码评审的效率。
创建小的提交列表
你可能已经听说过通过部署最小功能进行增量开发的好处。这样的一种部署减少了交付时间，提供了快速的反馈并降低了技术风险。当你将代码评审加入到增量开发中时，所带来的好处更加显著。当提交列表的大小增加时，等待评审者的反馈所花费的时间也会增加，并且大多数情况下会以超线性的方式增长。这有以下几个原因：
首先，当大的提交列表中存在设计缺陷时，它会影响你已经实现的更多代码段。但是，如果你将大的提交列表分解成几个小的提交列表并将它们一个接一个地合并，就可以避免一些返工。第二个挑战是将提交列表与主分支合并。当你在处理你的提交列表时，其他人也在处理他们的提交列表。因此，在检查代码时，很自然地会将一些提交列表合并到主分支中。如果其他提交列表与你的提交列表涉及到相同的地方，则合并将导致进一步的变更，这将导致额外的检查时间。当提交列表长时间保持打开状态时，它会同时惹恼开发人员和评审人员。它使他们丧失斗志，因为他们没有前进的感觉。这项工作不被认为是“完成了”，未针对业务显示出任何利润。
但是，将提交列表划分成更小的提交列表并不总是那么容易。主要API的变更、迁移到新的框架或基础设施、架构重构以及应该一起发布的特性只是我们面临的其中一些挑战。在这种情况下，特性标志和抽象分支（Branch By Abstraction ）等实践可能会有所帮助。
要使用这些方法，你可能需要在代码中引入切换标志，并创建模拟对象和额外的接口。尽管你可能认为做这些事情是在浪费时间，但大多数时候，通过减少研制周期并帮助你取得平稳可靠的进展，你会得到回报。
提交列表应该由他们自己独立完成
如何分解你的提交列表非常重要。例如，通过推迟单元测试来划分提交列表是不好的做法。虽然这是一个非常简单的选择，但是这种做法会使代码库处于不可靠的状态。这种状态会保留在代码中，甚至在临近截止日期、新特性优先于延迟的工作时变得更糟。因此，你应该尝试有一个小而完整的工作，包括它的测试，甚至它的部署/迁移脚本。
应该让最佳论点获胜
在出现分歧的情况下，哪一方应该获胜？是程序员还是评审人员？谁更有经验？没有人？事实上，这不是谁的问题！建立一种让最佳论点获胜的文化。
如果你是一个有经验的开发人员，你应该能够为你的论点提出理由，而不是仅仅表达你的“感觉”。如果你有某方面的经验，试着解释一下，为什么你认为它适用于当前的环境。因此，在最佳论点获胜的文化中，“我只有在有论据支持的情况下才会提出主张！”试试这个，你会受益于两个原因:
- 你不会因为没有理由的事情而让你的同事不高兴。
- 做好解释你的论点的准备会给你力量。它鼓励你查阅教科书，在网上搜索论文，询问其他专家和资深同事。最后，你要么确保你的观点，要么纠正你的假设。
你会学到一些东西。所以，每次你添加评论时，请解释你的理由，除非理由显而易见或之前已经被解释过。如果你不像同事那么经验丰富，如果一条评论的目的不明确，你应该问问“为什么”，这是你的权利。评论是一次学习的机会。在下一个提交列表中，你应该减少重复的缺陷。当轮到你来评审的时候，你可以给其他同事类似的评论。要做到这一点，你就得在不知道的时候问“为什么”。
如果你认为另一种选择更好，可以毫不犹豫地与你的评审人员讨论它的利弊。然后，让更好的论点（更有优势的解决方案）获胜。但是，准备好接受它，不管它是不是你的主张。事实上，这是对资深和初级的程序员和评审人员的好建议。做一个有激情的人，一个想知道“为什么”的人，一个渴望为自己的想法辩护的人，但也要对其他的可能性和解决方案持开放态度。
如果另一个解决方案和你的一样好，就不要争论了
作为一位实现者，如果你得到一条评论来重命名一个变量，但是你认为建议的名称是相似的，没有明显的区别：接受它。作为一名评审人员，如果你想要提出一个修改意见，但是你不能解释你的建议的明显优势：跳过它。你可能会想，“我的解决方案和同事的一样好。我为什么要放弃呢？”答案很清楚。你的假设是错误的。对你来说同样好的事情，对你的同事来说可能就不一样了。如果在你的加权系统中，这些选项是等价的，你就是那个可以容忍它并体现灵活性的人。所以这样做吧。把争论留到对你重要的事情上。
还有，永远不要数，“我让了他们两次了；现在轮到他们了。”计数游戏是一种破坏性的方法，注定要失败。两个人对数字的感觉是不一样的。他们可能会数一些他们关心的事项，但是你却不在乎，反之亦然。
不要在评论中抱怨
在评论和回应中，不要抱怨或责备，如果它不够清楚，就加上你的理由。评论本身可能就很让人犯难。你会和同事意见相左；你会在他们的工作中发现一个问题。所以不要让抱怨把事情变得更糟。
当你的队友阅读你写的话时，他们可能不会采用你寄希望的语气和力度。如果它是一个否定句，那么当他们读到这句话的时候，会觉得这是对他们的一种侮辱，或者这句话是带着轻蔑的态度写出来的，这也就不足为奇了。表情符号会有所帮助，但表情符号很难同时表达严肃和尊重！
删除表达负面情绪的词语。但是，当你看到一个很棒的设计或聪明的想法时，不要犹豫，要表现出肯定的感觉。
不要在评论中说“不”
尊重你收到的评论。如果你不同意，或者你不知道评论的目的，那就谈谈或者写下来。但是，不管你是谁，都不应该说“不”这个词。“不”这个词表示你不会接受别人的评论，也不会讨论原因。”“不”扼杀团队文化。
利用口头交流
使用评审工具并不会让你仅仅局限于书面交流。口头交流可以帮助你:
- 解决棘手和有挑战性的问题——这样的对话更具互动性，也更有效果。你可以避免就同一条评论进行冗长的书面争论，因为这只会惹得双方都不高兴。
- 当你对一些问题持有疑问时，避免进一步的评审。让评审人员坐在你旁边解决这些问题。
- 节省时间，在评审过程开始时就了解全局。如果你不熟悉提交列表的用途和设计，请实现者口头描述可以节省时间。
评审一切
如果你不是一个好的开发人员，你就不能成为一个好的评审人员。在开发过程中，你要考虑许多方面：符合需求、符合架构、符合代码风格和约定、符合以前的设计、设计和结构的简单性、可读性、避免冗余代码、低耦合、高内聚、清晰一致的名称、安全性、可伸缩性、高可用性等等。
有许多事情需要考虑，你也应该在评审时考虑所有这些事情。因此，评审技能包括所有的开发技能和一些更多的技能——交流、清晰的推理、倾听和指导。
让工具承担一些负担
正如你在前面的主题中看到的，有许多方面需要评审。但是，也有许多源代码分析工具可以帮助评审人员减轻一些负担。工具特别擅长检查代码风格和约定。它们还可以检测简单的bug和安全问题。如果你将简单直接的任务交给自动化工具，就可以专注于更深入、更具挑战性的问题。最近，我使用了SonarQube，这是最流行的分析工具之一。这样的工具不可能找到每一个问题，也不可能提供经验丰富的评审人员所能提供的每一种创造性反馈。根据我的经验，它只完成了不到5%的工作，但是它仍然可以通过自动化流程的某些部分来帮助评审人员。当评审人员知道将会检查很少的代码风格问题、编译器警告和几个已知的bug时，他们就可以集中精力进行更卓有成效的改进。此外，工具更适合于日常工作。如果一个工具就一条简单的规则去检查数千个变量、条件和循环语句，它不会觉得厌烦。
评审人员应该予以认证
开发团队中的每个人都可以成为评审人员吗？不！我认为技术经理应该认证评审人员。评审资格证表明开发人员已经掌握了产品的技术技能和业务方面的知识。有些人认为，当编码人员具有丰富的经验时，我们可以让一个平庸或缺乏经验的人来检查代码。我不这么认为，因为我把评审过程看作一个控制关卡——一个用来捕捉实现者遗漏的东西的监视系统。如果我们不尊重评审过程本身，我们怎么能鼓励人们尊重评审反馈呢？评审是一个鼓励初级开发人员掌握他们技能的机会。为最勤奋的人颁发一枚评审徽章，这会让开发人员为他们的成就感到自豪，并开始充满信心地进行评审。
优先考虑评审而不是编码
一边是你自己要开发的提交列表，一边是要评审的另一个人提交列表。你会优先选择哪一个？也许你的任务列表上没有代码评审。也许这个问题在跟踪管理系统上分配给了别人，你没有看到你的名字。不管怎样，这是别人的问题，你不需要直接负责这个提交列表。你可能会说，“我会在检查另一个提交列表之前完成我直接负责的工作。”对我的评价将主要基于我自己的提交列表。”
但是，这种做法是错误的。优先评审代码以减少正在进行的工作(WIP)。通常，当以前的提交列表正在等待反馈时，开发人员会启动一个新的提交列表。一次有两个，一个在等待评审，一个在开发中，这是一个合理的决定。但是，往往故事的结局不是这样的。如果评审人员没有对评审任务进行优先级排序，那么他们可怜的同事可能不得不启动第三个和第四个提交列表。在团队级别，这意味着更多已经打开的提交列表。更多已经打开的提交列表意味着团队将花费更多的时间来解决冲突，频繁地切换上下文切换带来更多的开销，最终更加延迟了使它们变得更有生产力和更能盈利，并从用户那里获得反馈的过程。
另一方面，优先评审有助于团队文化。我们都是一个团队的成员，我们都有一个共同的目标——为我们的客户和我们自己谋取利益。在这样的文化中，对每个人的评价都是基于他们为实现共同目标而进行的协作。
优先评审是一条规则，但不是一条严格的规则。当然，当你自己的提交列表中只剩下少量工作时，或者这个提交列表的优先级更高时，正确的做法是继续完成你自己的提交列表。但是，在其他时候，优先考虑评审是非常有利的。
调解员应遵循相同的文化
想象一下评审人员和编码人员之间的一个争论。双方都讨论了他们的观点，但他们不能得出相同的结论。一方说某事有优点或缺点；另一方要么不接受，要么认为还有其他重要方面导致了不同的结论。
即使当双方都认为两种解决方案都不错的时候，愿意采取灵活的态度，双方仍有可能坚持自己的观点。在这种情况下，请第三方(调解员)介入会有所帮助。关键是我们如何看待这位调解员的贡献。一如既往，“最佳论点应该获胜”，而不是个人。调解员应尽量集中于论点、推理、要点及其权重。我们不应简单地把这个人看作是一个选民或拥有否决权的人。相反，我们应该把这个调解员看作是一个队友，他能帮助我们就这个问题有一个更清晰的认识。使用这种方法，双方都可以“大致”满意于共同思考后得出的结论。有时，最终的解决方案甚至是一开始没有考虑到的。
与评审人员配对
追求代码评审的实践并不排除结对编程。结对编程已经证明了它的有用性，特别是当很难达到一个好的设计或找到一个解决方案的时候。在这些情况下，你可以向任何团队成员寻求帮助，但是最好的选择之一是与该任务的评审人员配对。除了得到帮助之外，你还可以将所花费的时间降到最低，并评审冲突。
这种配对不需要在提交列表期间继续。它可以根据需求而定。因此，当你对实现的某些方面有疑问时，或者当你对当前实现不满意时，就不要继续使用有问题的方法。与其等待正式的反馈，不如早点问问。
有效且令人愉快的代码评审
在技术(代码)和文化(交流)方面采用良好的实践可以使你的代码评审更加有效和愉快。它有助于代码质量、知识共享和团队文化。
作者介绍：
Mohammad Ali Bozorgzadeh是一名技术经理，他热衷于指导开发人员并提高组织中的技术成熟度。他的技术专长集中于大数据、敏捷方法和持续交付。他目前在Sahab Pardaz工作，这是一家在中东的公司，提供大规模和超大规模大数据解决方案。
原文链接：
- 发表于:
- 本文为 InfoQ 中文站特供稿件
- 首发地址：https://www.infoq.cn/article/yPfvTqgCdcvbTllzoY4k
- 如有侵权，请联系 cloudcommunity@tencent.com 删除。
相关快讯
扫码
添加站长 进交流群
领取专属 10元无门槛券
私享最新 技术干货
腾讯云开发者
