Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

publish-commit-bottles: various improvements #128697

Merged
merged 8 commits into from
Apr 19, 2023
Merged

Conversation

carlocab
Copy link
Member

@carlocab carlocab commented Apr 18, 2023

I pushed my changes from #128544 to a branch here to enable testing of changes to publish-commit-bottles.

Additional changes since then:

  • set concurrency in publish-commit-bottles based on feedback (fd30058)
  • remove unneeded debug output from tests.yml (7fdb1e8)
  • handle merging PRs with no bottles to publish (9e75776e15fcc3c912e25028fac111c48b122c06)
  • retry failed bottle merges (a871f22fc0b67228d2e81c787f813c4e0be11208)

@github-actions github-actions bot added automerge-skip `brew pr-automerge` will skip this pull request workflows PR modifies GitHub Actions workflow files labels Apr 18, 2023
@carlocab carlocab force-pushed the merge-on-approval-or-ci branch from a871f22 to 6f285ac Compare April 18, 2023 17:51
@carlocab
Copy link
Member Author

Seems to work: #128798

@carlocab carlocab marked this pull request as ready for review April 19, 2023 20:46
@carlocab carlocab requested review from a team and MikeMcQuaid as code owners April 19, 2023 20:46
ZhongRuoyu
ZhongRuoyu previously approved these changes Apr 19, 2023
Comment on lines 3 to 4
on:
workflow_run:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nice thing we can do with workflow_run events is that we can use them to remove CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. labels from PRs when CI finishes.

Using that to find another long build Set a long timeout for formula testing PR and re-running their CI is slightly more complicated, but may be possible.

@carlocab carlocab marked this pull request as draft April 19, 2023 22:29
Copy link
Member Author

@carlocab carlocab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not mergeable as-is, because the way our workflows currently run (@BrewTestBot dismisses reviews and then approves them) allows the possibility for merging PRs after an approving review from a user with no write access.

I'll break off the parts that don't have to do with automerge into a separate PR.

@carlocab carlocab changed the title workflows: publish approved PRs upon CI completion publish-commit-bottles: various improvements Apr 19, 2023
This will make sure we only run one of these at a time for any given PR.

While we're here:
- add stricter checks to make sure we don't attempt to `pr-pull` PRs
  that we don't want or need to
- avoid looping over arrays twice when we don't need to
- improve readability by relying less on workflow expression interpolation
- use `GITHUB_TOKEN` more often
These are no longer needed.
@ZhongRuoyu
Copy link
Member

Maybe we can check this to determine the validity of approvals:

# no approvals yet
$ gh api repos/Homebrew/homebrew-core/pulls/128697 -q .mergeable_state
blocked

# approved, with failing CI
$ gh api repos/Homebrew/homebrew-core/pulls/128536 -q .mergeable_state
unstable

# approved, with green CI
$ gh api repos/Homebrew/homebrew-core/pulls/128809 -q .mergeable_state
clean

@carlocab
Copy link
Member Author

Maybe we can check this to determine the validity of approvals:

Nice. I'll break that off into a separate PR. Most of the changes here no longer have anything to do with automatically merging PRs.

@carlocab carlocab marked this pull request as ready for review April 19, 2023 22:42
We cannot approve PRs using a @BrewTestBot PAT, because this doesn't
work for PRs opened by @BrewTestBot.

Similarly, we cannot merge PRs using `GITHUB_TOKEN` because this will
not trigger our required `merge_group` checks.

So, instead of using one token for both actions, let's approve PRs using
`GITHUB_TOKEN` and merge PRs using a @BrewTestBot PAT.
@carlocab carlocab force-pushed the merge-on-approval-or-ci branch from 6c1064c to 5448c89 Compare April 19, 2023 22:48
@carlocab carlocab added this pull request to the merge queue Apr 19, 2023
Merged via the queue into master with commit 8a4bd54 Apr 19, 2023
@carlocab carlocab deleted the merge-on-approval-or-ci branch April 19, 2023 23:31
@@ -2,6 +2,10 @@ name: Publish and commit bottles

run-name: "Publish PR #${{ inputs.pull_request }}"

concurrency:
group: ${{ github.workflow }}-${{ inputs.pull_request }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like inputs.pull_request is showing up as blank here, so we're actually running all these jobs serially at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip `brew pr-automerge` will skip this pull request workflows PR modifies GitHub Actions workflow files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants