-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Description
Verification
- This issue's title and/or description do not reference a single formula e.g.
brew install wget
. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.
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:
- Do nothing and keep submitting duplicate PRs. This seems non-ideal in terms of noise/maintainer overhead, but we could do it.
- 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 consumingdev-cmds
) and would have larger external API compat implications. However, if people prefer this route, I can go this way instead 🙂
CC @alex