Skip to content

GitHub::check_for_duplicate_pull_requests should have a strict failure mode #19828

@woodruffw

Description

@woodruffw

Verification

Provide a detailed description of the proposed feature

Filing this for myself to implement once I have a moment 🙂

TL;DR: I'd like to add a strict kwarg to check_for_duplicate_pull_requests that restores the "fail hard" behavior. This kwarg will be false by default to make it compatible with the current behavior, while consumers like brew-pip-audit can set it to true to restore their previous API expectations.

What is the motivation for the feature?

check_for_duplicate_pull_requests provides the duplicate PR checking functionality for a bunch of dev commands, as well as for brew-pip-audit.

Starting with #18206, the behavior of check_for_duplicate_pull_requests changed slightly: before it would fail hard (with an exit) if a duplicate existed, and after it only fails hard if being run on an official tap and the version kwarg is present. In all other cases, the duplicate check now fails "softly" by emitting a warning.

This works well for the common use case (deduplicating version bump PRs), but it unfortunately means that brew-pip-audit now creates duplicate PRs some of the time, e.g. Homebrew/homebrew-core#221601 and Homebrew/homebrew-core#221607.

How will the feature be relevant to at least 90% of Homebrew users?

This will make brew-pip-audit less noisy (like it was before), which in turn will reduce maintainer load/manual work when triaging brew-pip-audit PRs.

What alternatives to the feature have been considered?

Alternatives:

  1. Do nothing and keep submitting duplicate PRs. This seems non-ideal in terms of noise/maintainer overhead, but we could do it.
  2. Refactor check_for_duplicate_pull_requests more broadly to be a "pure" API, i.e. use exceptions and return types to signal its status instead of emitting warnings and failing directly. This is probably conceptually cleaner, but it requires a more intensive set of changes (e.g. within all of the consuming dev-cmds) and would have larger external API compat implications. However, if people prefer this route, I can go this way instead 🙂

CC @alex

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedWe want help addressing this

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions