我们只需4/30即可调整Futureestack注册。条款和条件适用。 现在注册

为代码评论创建简单效率的准则

12分钟阅读

这篇文章适用于给出的谈话Futurestack18旧金山标题为“代码评论的地面规则”。

追随新的遗物项目高档-an创新重组旨在使我们的开发团队更加自主 - 工程组织形成了几个新的团队,其中一个是新的遗物数据库(NRDB)团队。顾名思义,NRDB团队负责开发我们的活动数据库,这为新的遗物见解工具以及其他几种产品。亚博最新版直播

当我们组建NRDB团队时,它包括多个高级软件工程师。这是一个高技能和非常激情的开发人员,审查了彼此的拉出请求。

当激情变得有毒

对工作充满热情是New Relic的核心价值观之一。然而,在这种情况下,我们可能经历了太多的好事:我们的代码审查很快就成为了冲突点,我们越来越多地转向被动攻击的沟通来解决我们的分歧。

在代码审查期间发表评论
在左侧的示例中,审稿人将PR留在与之间的状态。他们没有明确拒绝它,但他们也没有批准它。在右侧的示例中,审稿人发出了一个非常主观的请求,作者刚刚变化,但从他们的语气来看,你可以猜测他们不欣赏反馈。

结果,NRDB团队的开发人员越来越沮丧,团队信任受到侵蚀,一些成员(包括我自己)考虑转到其他团队。我们有麻烦了。

炼制我们的过程 - 并拯救团队

我们决定作为一支努力返回;我们决定弄清楚正在发生的事情,为什么发生了什么,以及我们可以做些什么来解决它。由于我们大多数挫折与我们的编码审查相关联,我们开始询问一个简单的问题:我们如何给另一个更有效和建设性的反馈?

我们通过为代码评审制定四个基本准则来回答这个问题。我们认为你也会发现它们是有用的,但在我们解释之前,我们想要分享整个故事背后发生的事情,划分我们的团队和什么对我们来说是真正的赌注。

代码审查流程的缺陷方法

我们的许多挑战与我们的编码审查中目标和主观反馈之间的差异有关。能够在这两种类型的反馈之间清楚地区分,对代码审查的成功以及开发团队的有效性来说至关重要。在太多情况下,我们没有以建设性的方式处理主观反馈 - 实际上,相反是真的。

我们可能不是唯一为这个问题而挣扎的人。许多开发人员都从开始训练淡化两种反馈之间的差异。事实上,学术软件工程项目的学生很少学习如何给予或接收任何类型的批判性反馈。

当我上学时,这肯定是这种情况。计算机科学课程专注于算法分析,数据建模和解决问题。我们的教师将代码审查视为功能质量保证任务;他们很少把它作为一个有创造力的过程。代码审查反馈往往是简单的:代码有效,或者它没有。由于这种培训 - 或者相反,缺乏培训 - 许多软件工程师仍然将编码审查的所有方面视为完全客观的活动。

将这种方法与学术创意写作计划的专业人员造影相比,这是有用的。在那里,教师进行包括培训的研讨会,了解如何提供批判反馈。创意写作教练了解给予和接收关键反馈是创意过程的重要组成部分。然而,他们还了解,除非正确处理,否则批判性反馈可能是有害的并且创造怨恨。目标是以积极和建设性的方式提供反馈,有助于磨练作家的想法,增强他们的创造力,并留下富集的两党。

对主体性的斗争

然而,代码审查的许多方面并不简单。特别是,有些问题需要主观评估,而没有“正确”答案。这就是为什么严格强调代码审查是一个完全客观的活动,而不考虑软件开发的创造性本质,会成为一个问题。

现代化的代码审查过程的许多元素现在完全自动化。编辑和IDES将找到语法错​​误,评估布尔逻辑,并警告无限循环。因此,生存的错误更难找到,特别是当您在进程结束时,只是在查看具有有限背景的代码片段。

但是,编辑器和IDE无法检测到 - 或阻止开发人员关注主观问题,例如令人困惑的方法名称,可疑的样式偏好以及不变的变量格式。当我们不喜欢和不同意我们在这种情况下发现的时候,我们常常忘记这些“缺陷”是意见的主观问题 - 而不是客观事实。

这种方法也很容易让人忘记在代码评审过程中关于主观问题的争论会很快变得情绪化和激烈。

在代码评审期间提出的意见

有些团队通过创建摆脱主观偏好的风格指南,尝试在存在的情况下规范这个问题。这种方法很少成功:软件开发充满了主观选择,没有办法涵盖开发人员在项目过程中可能面临的每个主观选择。

当一个团队缺乏明确的通信通道进行主观反馈时,问题变得更糟。审核人员可以在不承认差异的情况下混合主观和客观评论;这里也可以在怨恨,挫折和团队沟通中结束。

我们的代码审查的四个准则

这使我们返回我们开发的指导方针,以管理NRDB团队代码审查流程的主观要素。

首先,作为我们四个指导方针的初步,我们同意定义谁最终对代码变更的正确执行负责。这对我们很重要,因为在一个主观辩论中,那些拥有最终责任的人的意见 - 换句话说,核实守则应该携带最大的重量。结果,我们决定“代码变更的作者负责正确执行变更。”

这似乎是显而易见的,但并非所有团队都这样做。例如,某些团队将审核过程视为QA进程,审阅者最终负责验证正确执行。

我们发现主观评论最常被呈现为流程的拉请求阶段的客观反馈。因此,这是我们专注于我们的代码审查指南。

在创建这些规则时,我们为团队成员奠定了基础,以清楚地确定代码审阅者应该寻找的内容,以及如何给出主观和客观反馈。以下是指导方针:

  1. 审阅者应识别将导致生产问题的错误。这是一个代码审查,毕竟,审阅者应识别丢失的分号,无自由循环或缺少错误处理。审稿人不责任找到全部这样的错误(这仍然是作者的责任),但如果它们部署到生产,他们应该在寻找明显的问题上寻找明显的问题。这些问题是阻止拉拉请求的有效原因。
  2. 审阅者应验证代码更改的规定目标是否与所做的更改对齐。如果作者提出了一个拉拉请求,这些提出了他们对服务的网络代码进行了更改,则审阅者应期望所有更改都在服务的网络代码中及其周围。这似乎是显而易见的,但是没有秘密的秘密,开发人员在这种情况下尝试包装多种变化的倾向。这甚至不是必然是错误的做法,只要改变大多是共同定位的。但是,当您对其规定的目标对齐代码更改时,您可以更轻松地确定拉请求是否可能提交任何新错误。在这里,我们同意通过其规定的目标与代码更改保持对齐,这将是证明阻止拉拉请求。
  3. 审阅者应验证是否与团队的编码标准对齐。我将更多地覆盖这一点,但作为一个例子,如果团队决定所有变量必须使用骆驼盒,并且审阅者找到不使用骆驼盒的变量,它们应该阻止拉出请求。
  4. 审稿人应该寻找他们个人不同意的任何事情。本指南讨论了前三个规则不涵盖的任何评论。我们审阅者要给出反馈,即使这不在前三条规则的范围内。我们不希望我们的指导方针压制反馈,这是我们如何相互学习的关键。然而,因为这些评论显然是主观的,所以我们一致认为它们不能成为阻止pull请求的理由。

要删除所有混淆,我们要求审阅者特别呼出他们的评论,因为阻止或非阻塞;并将这些评论添加为您的评论中的标签。例如:

客观评论

  • “阻止:你缺少分号。”
  • “阻止:这个循环永远不会结束。”
  • “阻止:您缺少一些错误处理”

主观的评论

  • “非阻塞:您的方法名称不够清晰。”
  • 非阻塞:你应该把左花括号放在上面的行上。
  • “非阻塞:您应该在此处使用骆驼盒,而不是蛇盒。”

在我们的代码审查指南内工作

在我们采用这些指导方针时,该团队最困难于第四个。采用这意味着我们必须接受两个条件:

  1. 我们团队制作的代码不需要是统一的。这意味着克服了我们行业的趋势,说您应该努力从您的代码中删除所有指纹,以识别谁写的部分。我们发现投资回报率跟随这一趋势非常低,并试图这样做,只是让我们回到同样的主观辩论中:如果开发人员以略有不同的方式写作的代码比他们的同伴略有不同,那么代码是否意味着代码不正确?显然,我们决定,那不是客观的案件
  2. 如果审阅者添加非阻塞反馈,提交人应该花时间考虑。早期,一些团队成员感到关切的是,作者将只是忽略所有非阻塞评论,因为他们的代码不再被主观反馈阻止。然后,我们的解决方案是重申“我们相信我们的队友。“如果作为审稿人,我们花了时间进入评论,我们信赖作者将花时间阅读并考虑它。

这些都是争议的积分,而且团队花了很长时间辩论它们。但最终,我们发现,通过这些问题成功地工作的唯一方法是与指南一起生活并给予他们机会。

赞助编码标准

所以,如果一个审稿人看到一些他们热情地感觉不应该出现在代码中,特别是如果他们关注的不是可以阻止的“客观”规则违反?对于这些问题,我们同意审稿人可以选择赞助一个对团队编码标准的补充。

每两周,我们举行一次回顾会议,欢迎团队成员提出修改或增加我们的编码标准的建议。这个活动有两个限制:

  1. 我们无法描述主观语言的编码标准。例如,赞助商不能说,“变量不得暧昧”,因为歧义是主观的。但是,赞助商可以添加一个标准的状态,“变量必须使用匈牙利符号”,因为这是客观且易于执行的。
  2. 一世如果你赞助了一个编码标准,你必须支持它。赞助商必须根据需要提供文件和培训。如果团队需要安装插件或其他工具,则赞助商负责支持它。这种限制确保了赞助商对他们想要添加到团队编码标准的任何内容的热情。

在编码评论中找到尊重和妥协

在同意这些指导方面之后,我们清除了所有现有的编码标准并开始结束。在最初的几周里,很难打破旧习惯,我们不得不提醒几个团队成员在拉出请求评论期间添加阻塞和非阻塞标签。但是一旦我们与新的准则进行了滚动,我们就会看到了一些成功。

首先,迫使审稿人清楚地确定具有主观性的评论,我们注意到审核人员如何对他们的评论进行改变。评委者不能再满足他们偏好的改变;相反,他们必须礼貌地要求更改,并解释为什么他们要求改变。当我们以这种方式提供更多的解释和上下文时,我们创建一个环境,使队友更容易彼此学习。加,询问对于变化,而不是要求他们,表明尊重和承认代码的作者以及对他们的工作的有效感受。

我们还注意到,当审稿人确实写了一个不封锁的评论要求改变时,作者通常会提出所要求的改变或提出妥协 -即使作者有选择忽略评论。这证明了为什么要求改变,而不是要求他们,建立更强大的团队:作者感觉不那么怨恨,审稿人认为作者真正欣赏他们的反馈。

我们已经从这个过程中发现了一些其他的好处。例如,通过限制阻塞注释的范围,我们减少了批准和合并变更所花费的时间,从而提高了整个项目的速度。我们还减少了招募新的团队成员以及让他们快速适应我们的代码审查过程所需的时间。

我们还更新了我们的培训材料,以反映我们新的代码审查过程:我们发布了一页记录我们的指导方针,另一页记录我们的编码标准。新的团队成员现在确切地知道他们应该寻找什么,以及如何最好地传达他们的建议。

我们还预计随着审阅人员赞助商品的新标准,我们还预计将增加编码标准的数量,他们无法再阻止。一开始,我们确实采用了几个新的编码标准,但在初始爆发后,新协议的数量明显脱落。我们得出结论,由于审稿人士认为提交人正在考虑到他们的主观反馈,因此根据他们的观点,他们并没有认为“将”它们“转换”它们转换为客观的限制。

提出的提案数量:预期与实际

这些指导方针有助于团队自治

这些指导方针最重要的是他们支持团队自治;这些指导方针不能决定哪些编码标准团队应采纳。团队可以自由选择自己的风格指南,他们决定他们想要的严格。这些指南简单地解释了如何定义编码标准以及审阅者应该如何寻找并提供反馈。

我们来欣赏强大有效的反馈过程可以对建立团队士气,增加团队信任和沟通以及提高发展速度的作用。我们实施了加强反馈过程的指导方针,并解决了在风险风险下进程的问题 - 到目前为止,我们认为我们正准确地从这些改进中获得了什么。