-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
🤖 Created branch: z_pr148/skitt/merge-groups |
You have successfully added a new Grype configuration |
tim-actions/get-pr-commits doesn't seem to support the merge_group event. |
DCO also doesn’t support merge queues: dcoapp/app#199 |
The relevant get-pr-commits code is: https://github.com/tim-actions/get-pr-commits/blob/master/index.js#L4 I'll send a PR to add the merge_group event. |
|
Thanks! It’s not all that useful for our specific scenario, but I imagine other users will need this. (For us, since the PR’s commits are checked before the PR is added to the merge queue, and the merge queue won’t change the PR’s commit titles etc., we don’t need to re-check them in the merge queue.) |
See https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue for details. The target_devel and apply-suggestions-commits jobs are required, so they need to run on merge group events, but their results are only valid outside the merge queue, so they are ignored if the event isn't a pull_request event. The DCO app is required by the CNCF, but doesn't support merge queues yet. Until it does, this restores the old DCO check for PRs, ignoring it in merge queues; that way, we can require that, ensuring that PR commits have a DCO, without blocking the merge queue. (The general reasoning is that PR checks should verify everything related to the commits themselves; merge queue checks should verify everything that changes as a result of the position of the PR in the queue, which doesn't include commit texts, so anything related to that can be ignored in the merge queue if it's been checked in the PR. GitHub doesn't support different requirements for PRs and merge queues yet, so this needs to be handled by skipping irrelevant tests in merge queues in the workflow configuration itself.) Signed-off-by: Stephen Kitt <skitt@redhat.com>
The relevant GitHub discussion for MQ v. PR requirements is https://github.com/orgs/community/discussions/46757#discussioncomment-4908640. |
🤖 Closed branches: [z_pr148/skitt/merge-groups] |
See
https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue
for details.
The target_devel and apply-suggestions-commits jobs are required, so
they need to run on merge group events, but their results are only
valid outside the merge queue, so they are ignored if the event isn't
a pull_request event.
The DCO app is required by the CNCF, but doesn't support merge queues
yet. Until it does, this restores the old DCO check for PRs, ignoring
it in merge queues; that way, we can require that, ensuring that PR
commits have a DCO, without blocking the merge queue.
(The general reasoning is that PR checks should verify everything
related to the commits themselves; merge queue checks should verify
everything that changes as a result of the position of the PR in the
queue, which doesn't include commit texts, so anything related to that
can be ignored in the merge queue if it's been checked in the PR.
GitHub doesn't support different requirements for PRs and merge queues
yet, so this needs to be handled by skipping irrelevant tests in merge
queues in the workflow configuration itself.)