Use gh to automerge and cleanup branches on bump* PRs.
#15784
Conversation
MikeMcQuaid
requested review from
chenrui333,
razvanazamfirei,
carlocab,
p-linnane and
ZhongRuoyu
This should allow reducing the mass PR creation flow by a few clicks. Fixes #14538.
| @@ -58,6 +61,7 @@ def bump_cask_pr_args | |||
| conflicts "--no-audit", "--online" | |||
| conflicts "--version=", "--version-arm=" | |||
| conflicts "--version=", "--version-intel=" | |||
| conflicts "--" | |||
There was a problem hiding this comment.
❓
| conflicts "--" |
There was a problem hiding this comment.
Whoops, needs updated.
| rescue *API::ERRORS => e | ||
| odie "Unable to open pull request: #{e.message}!" | ||
| end | ||
|
|
||
| # Attempt to enable automerge on the created PR if requested. | ||
| GitHub::API.run_gh_cli(args: ["pr", "merge", "--auto", "--merge", "--delete-branch", url]) if args.automerge? |
There was a problem hiding this comment.
Is there a reason we do this rather than call the API directly like we do everywhere else?
This sounds like this would not work with HOMEBREW_GITHUB_API_TOKEN.
Code using the GraphQL directly (untested but should work in theory if no typos):
pr = create_pull_request(...)
url = pr["html_url"]
# ...
if args.automerge?
API.open_graphql(<<~EOS, variables: { pullRequestId: pr["node_id"] })
mutation($pullRequestId: ID!) {
enablePullRequestAutoMerge(input: {pullRequestId: $pullRequestId, mergeMethod: MERGE}) {
clientMutationId
}
}
EOS
end
There was a problem hiding this comment.
See also similar-ish code in Homebrew/core: https://github.com/Homebrew/homebrew-core/blob/f0c6e070add5312cb25639bcade628c21a6de2af/.github/workflows/publish-commit-bottles.yml#L211-L232
There was a problem hiding this comment.
@Bo98 pretty much because:
- we already have code to shell out to
gh - we already rely on
ghfor homebrew/core - honestly for anything that's not performance-critical: I'd rather we pushed more of our code into
ghcalls than doing direct API access.
If you don't consider this a hard blocker: would rather land this as-is to validate the idea/workflow and anyone who wants to can move to GraphQL later if that's desired.
There was a problem hiding this comment.
we already have code to shell out to gh
This appears to be the first beyond token fetching?
I'm not entirely convinced it'll work in Homebrew/core CI as is given how our tokens appear to be used over there (plus that additional issue Carlo pointed out), but have of course not tested it. Maybe Homebrew/core CI is a non-concern anyway judging by the original issue.
There was a problem hiding this comment.
This appears to be the first beyond token fetching?
Yes.
I'm not entirely convinced it'll work in Homebrew/core CI as is given how our tokens appear to be used over there
Yes, may require extra wrangling for the bump job(s).
Maybe Homebrew/core CI is a non-concern anyway judging by the original issue.
Indeed, this may be rescoped to cask only.
| switch "--automerge", | ||
| env: :bump_automerge, | ||
| description: "Open pull requests with automerge and branch cleanup enabled." |
There was a problem hiding this comment.
Not sure this works as-is yet, since bump-formula-pr PRs will still need a bottle commit pushed, so I'm not sure we want to enable automerge on them. In theory doing so should do nothing given the bottle checks in the merge queue though.
There was a problem hiding this comment.
@carlocab Oops, I've misunderstood the workflow. Does this make this PR basically pointless for homebrew/core (and/or homebrew/core)?
There was a problem hiding this comment.
At the moment, it is pretty pointless for Homebrew/core. I can see this being useful if/when PRs are blocked from being queued to merge if they are still missing a bottle commit (and require one).
There was a problem hiding this comment.
@Homebrew/cask would this be useful for homebrew/cask?
There was a problem hiding this comment.
Cask has automation for this I believe
There was a problem hiding this comment.
Yeah, homebrew/cask* already auto-enables automerge for all maintainer-authored PRs labelled bump-cask-pr: https://github.com/Homebrew/homebrew-cask/blob/master/.github/workflows/automerge.yml.
Example: Homebrew/homebrew-cask#152038
There was a problem hiding this comment.
Cool, problem solved then, thanks all.
This should allow reducing the mass PR creation flow by a few clicks.
Fixes #14538.