Improve duplicate pull request handling

[MikeMcQuaid]

• change the messaging depending on how confident we are that we're actually looking at duplicates i.e. we're not confident without a version number supplied
• similarly, just warn instead of failing with an error (and no override) if we're not confident that we're looking at duplicates because a version wasn't supplied
• change bump-cask-pr and bump-formula-pr to always check for all pull requests with the new version number (to allow failing on this) rather than only checking closed pull requests with a version number
• change bump to check for definite/maybe duplicate PRs and only exit if they are definitely duplicates
• cleanup some variable usage to DRY things up a bit

Inspired by conversation in #18205

[carlocab]

Makes sense, I think.

Any reason why we have different handling for bump-cask-pr and bump-formula-pr?

Also, do we not need changes for dev-cmd/bump.rb too?

[MikeMcQuaid]

Any reason why we have different handling for bump-cask-pr and bump-formula-pr?

@carlocab Can you elaborate? They should be functionally equivalent, they just use different code paths to get there. I'm deliberately trying to avoid changing behaviour beyond just the targeted change here.

Also, do we not need changes for dev-cmd/bump.rb too?

It doesn't call GitHub.check_for_duplicate_pull_requests? What did you have in mind there?

[carlocab]

They should be functionally equivalent, they just use different code paths to get there. I'm deliberately trying to avoid changing behaviour beyond just the targeted change here.

Yes, sorry, I was confused by the cosmetic differences!

Also, do we not need changes for dev-cmd/bump.rb too?

It doesn't call GitHub.check_for_duplicate_pull_requests? What did you have in mind there?

You're right that it doesn't, but the problem that motivates #18205 isn't just because of GitHub.check_for_duplicate_pull_requests.

In particular, my impression is that when BrewTestBot fails to open a version bump PR in Homebrew/core, GitHub.check_for_duplicate_pull_requests is never even called.

We can that in this workflow run. Here, we see BrewTestBot calling brew bump, which identifies a candidate duplicate PR (but isn't an exact match for the version). From what I can tell, it never even tries to call brew bump-formula-pr, or else we'd see an error message about duplicate pull requests. So it seems to me that changing GitHub.check_for_duplicate_pull_requests, bump-formula-pr, and bump-cask-pr is not sufficient here.

Does that make sense?

[MikeMcQuaid]

@carlocab makes sense but: ugh, this is why I like consistency. I'll likely port brew bump's PR calls to use the same methods.

[carlocab]

makes sense but: ugh, this is why I like consistency.

Same! But this is part of why I wanted to punt; the code is kinda all over the place.

[MikeMcQuaid]

I'll likely port brew bump's PR calls to use the same methods.

I didn't but: I should have fixed the brew bump case you mentioned.

[MikeMcQuaid]

Let's give this a go.