ITPub博客

首页 > 人工智能 > 人工智能 > 用 `small-change` 加速代码审查

用 `small-change` 加速代码审查

人工智能 作者:ITPUB社群 时间:1970-01-01 08:33:46 0 删除 编辑

Imprint 的普通工程师现在合并的 PR 数量是两年前的 4 倍。太好了。在平均值背后,还有更值得关注的情况:我们有两个团队在同一时期内合并的 PR 数量达到了原来的 10 倍。有一位工程师每周合并的 PR 数量是两年前普通工程师的 25 倍,还有几位也正在接近这个数字。这相当不错,而且从趋势线的斜率来看,我们一年后的平均值将更接近今天的 p90,而不是今天的 p50。更重要的是,我们不仅在交付更多,质量也有所提升:事故数量环比下降了 42%,严重事故环比下降了 78%。我想我们一直都知道质量与速度之间的权衡并不是一个特别有用的框架,但今天这一点已无可否认。

沿着这条路走下来,我们——和其他所有走这条路的人一样——遇到了拉取请求审查的问题。两年前,p50 拉取请求审查大约需要 33 分钟完成,但现在需要约 77 分钟。这里的斜率与合并的拉取请求数量紧密相关。相关程度之高,以至于解决这个问题成了我本周的优先事项之一。

我们解决了!开个玩笑,我们绝对没有解决,但我认为我们目前的做法很有意思,我们称之为 small-change。

本文讨论的工作由 Imprint 的许多人共同塑造和完成,包括 Dan、Joy、Daniel、Lenny、Taylor、Ted 和 Annie 等众多同事。

为什么要审查拉取请求?

我们审查拉取请求有四个原因:

  • 作者与审查者之间的知识共享——软件工程通常通过师徒制学习,而审查是经验丰富的工程师教他人什么是好软件的主要机制。我们认为,当前的规模意味着这种知识共享将不得不转向针对拉取请求集群进行操作,而不是单个拉取请求
  • 防止错误——审查一直是捕获错误的高成本时机,我们认为有效的 CICD 以及像 poteto 的 pstack 这样的 harness 验证技术,是比审查更有效的防错机制
  • 防止恶意行为——根据我的经验,即使是仔细的代码审查通常也会漏掉有能力的恶意行为,不过幸运的是大多数恶意行为并不高明。我们认为这两种行为最好通过组合方式来识别:固定规则(修改这些特定对象/类/文件需要审查)、我们在基于 LLM(大语言模型) 的会员支持体验中完善的防滥用机制、审计以及审计日志检查
  • 我们文档化控制措施中的合规义务——这些可以更改,但目前它们要求由独立的人工审查我们部分仓库中的变更

在现代开发范式中,我们认为每个问题都有更好的解决方案。但是,我们也希望非常谨慎地做出这些改变。

用 small-change 迈出小步

我们相信,通过找到狭窄、受控的方式来验证新实践,我们就能快速前进。因此,为了迭代出有效的解决方案,我们开发了 small-change 的概念。其工作方式如下:

  • 当拉取请求被标记为就绪时,CICD 会判断它是否有资格成为 small-change。这表现为一个通过的 Github 检查。还有一个用于可见性的标签,但决定资格的是由构建管理且不可人工修改的检查本身。(标签用于初步筛选,但检查仍会独立验证。)
  • 满足以下所有条件即为 small-change:代码少于 1,000 行 我们的 ai-code-review 自动审查机器人没有 p1 或 p2 级别的发现 作者在该服务或应用中具有“经验”,定义为之前有 10 个以上合并的拉取请求进入该服务或应用的目录。对于跨多个服务或应用的变更,作者必须在所有这些服务或应用中都有经验才能符合条件 影响少于 N 个不同的服务或应用。N 在我们的前端 monorepo 中为 1,在后端 monorepo 中为 3,因为典型的变更会同时修改一个 gRPC 服务和一个公共 HTTP 网关 共享库和工具文件不符合 small-change 条件 每个仓库都有一个 .small_change 文件,允许按路径自定义,以便我们继续调整行为。例如,处理资金流动的服务可以被排除
  • 我们的 Slack 自动化会为每个拉取请求引用打上 reacji 标签,标明该拉取请求的状态,例如测试失败、已批准但测试仍在运行、或已准备好合并。我们添加了一个 small-change reacji,用于指示哪些拉取请求符合 small-change 标准
  • 在我们的内部技能仓库中,我们创建了两个已分发到各 harness 的技能:review-small-change,它接收仓库列表并审查所有开放的、非草稿的 small-change,包括在满足审查标准时提供批准(所有检查通过,包括 small-change 检查,且不涉及支付和安全等敏感主题)。重要的是,这个技能只有两个选项:批准或忽略。它从不在 PR 上评论,以避免“肉代理审查”的泛滥——这在一些团队中已成为一种伪装成解决方案的瓶颈 review-pr-cluster,它审查批量拉取请求以识别出现的共同主题,并建议讨论和添加验证的机会
  • 我们设定了社会期望:任何由 small-change 引起的错误都是我们系统的失败,必须在系统层面解决:改进 ai-code-review、改进我们的验证机制、改进我们的 CICD、改进我们的 harness 等。

这花了大约两天的不定期关注来实施和推广,尽管我想我们会长期调整这里的参数。也许永远。

人工代理审查

我在上文 4.1 节中提到过这一点,但人工代理审查已成为许多团队一个有趣的瓶颈。这是一个特别棘手的瓶颈,因为它最初看起来像是一个解决方案,但实际上却让事情变得更糟。其模式大致如下:

  • 团队被审查工作压得喘不过气
  • 团队决定用本地测试框架来审查拉取请求
  • 拉取请求收到大量评论,有些合理,有些则不合理。不经过研究很难分辨哪些是合理的,而且你也不知道哪些评论反映了审查者的真实观点。因此,你最终不得不花费大量精力去反驳那些审查者可能自己都不同意的评论
  • 然后这些团队开始使用测试框架来回应测试框架生成的评论,使整个审查过程变成一种假象,既不利于学习,最重要的是,这些工作完全可以在没有其他团队成员参与的情况下完成

在每一个进行人工代理审查的案例中,如果团队将精力投入到集中式审查机制、用于处理集中式审查反馈的测试框架循环,以及简单地盖章批准上,他们都会获得更好的结果。

fleet:集中编排的测试框架

我将简要提及我们减少代码审查量的另一种方法,它以我们集中编排的测试框架 fleet 为核心。(这与 Stripe 的 Minions 类似,但并未特别遵循一次性方法)。我们依赖 fleet 在云端运行的高度受控测试框架中实现和验证软件,并通过独立的评估机制来确定任何给定变更所需的独立审查者数量。对于被评估为安全的变更,该数量可以低至一个(例如,fleet 编写,一个人审查)。

结果 / 后续步骤 / 反馈

说实话,现在给出有用的结果还为时过早。从早期结果来看,大约 10% 的变更符合小变更的标准。这些变更现在有资格获得非常快速的批准,并且通常都能获得批准,除非它们触发了某个标记,通常是因为涉及安全或资金相关主题。

虽然 10% 是一个很小的比例,但这里的核心概念正在发挥作用,最终成败取决于我们能否在未来几个月内扩大这一比例,同时扩展我们的验证机制使其越来越全面。这最终将取决于我们改进验证的能力,而在低规模复杂性、高领域复杂性的软件中,这确实需要比在高规模复杂性、低领域复杂性的产品中更困难的技术才能实现——在后者中,许多技术(如增量发布)异常有效。(别误会,它们仍然有效,只是当大多数用户具有不同的配置和行为时,它们提供的覆盖范围较小。)

人们对此并非一致感到兴奋,这与我们学习成为优秀工程师的方式有很大不同,重新评估一项长期以来的最佳实践是否已沦为教条,这让人感到不安。当前周期最令人振奋的部分是,我们每周都能重新评估这些伴随我们成长的做法;当前周期最痛苦的部分是,如果我们停止重新评估它们,我们很可能会落后。


转载声明:本文为转载整理,原文版权归原作者及原站点所有。

原文来源:Will Larson

原创地址:https://lethain.com/small-changes/

原文作者:Will Larson;如有侵权,请联系管理员删除。

请登录后发表评论 登录
全部评论

注册时间:2022-11-21

  • 博文量
    1199
  • 访问量
    549737