-
Notifications
You must be signed in to change notification settings - Fork 15
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
build: fix auto merge with merge queues #1213
Conversation
workflow_call: { } | ||
|
||
concurrency: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 Added this as a nice to have copied from the Zeebe CI. I can remove it of course, but it's generally useful to avoid having multiple runs for the same commit.
.github/workflows/build-test.yml
Outdated
@@ -178,13 +192,15 @@ jobs: | |||
- id: approve-and-merge-dependabot | |||
name: Approve and merge dependabot PR | |||
if: github.actor == 'dependabot[bot]' && (steps.metadata.outputs.update-type == 'version-update:semver-patch' || steps.metadata.outputs.update-type == 'version-update:semver-minor') | |||
run: gh pr review ${{ github.event.pull_request.number }} --approve -b "bors merge" | |||
run: gh pr merge --auto --merge "$PR_URL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ This won't work:
This enables the pull request to be merged when any tests and approvals required by the branch protection rules are successfully met
You removed the approval, so when you enable auto-merge it will only start the merge once someone gives approval.
Haven't tested the auto merge though, but I can keep an eye in the next few days for it. The merging itself looks like it will mostly work, though we can use this PR as a test (but at least the required check is green here, so it picked it up properly). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @npepinpe. I think there's a bug in the CI
.github/workflows/build-test.yml
Outdated
@@ -178,13 +192,15 @@ jobs: | |||
- id: approve-and-merge-dependabot | |||
name: Approve and merge dependabot PR | |||
if: github.actor == 'dependabot[bot]' && (steps.metadata.outputs.update-type == 'version-update:semver-patch' || steps.metadata.outputs.update-type == 'version-update:semver-minor') | |||
run: gh pr review ${{ github.event.pull_request.number }} --approve -b "bors merge" | |||
run: gh pr merge --auto --merge "$PR_URL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ This won't work:
This enables the pull request to be merged when any tests and approvals required by the branch protection rules are successfully met
You removed the approval, so when you enable auto-merge it will only start the merge once someone gives approval.
Oh, right, I assumed the auto flag would do that. |
78befb1
to
61a0a70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Description
Fixes the CI workflow, specifically enabling it for merge queues (fixing the triggers to run) and also updating the auto merge job to use the GitHub CLI to just add to the merge queue.
Related issues
related to camunda/team-infrastructure#633
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
Testing:
Documentation: