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

feat: add support for merge queues (merge_group events) #100

Merged
merged 7 commits into from
Sep 6, 2023

Conversation

janeklb
Copy link
Contributor

@janeklb janeklb commented Jul 31, 2023

Motivation:
nx-set-shas lacks support for github's merge queues

Modification:

  • when eventName === 'merge_group' and merge-queue-base-from-pr input set to true, set the BASE_SHA to the merge-base of the the default branch + PR's branch

Notes / disclaimer:

  • extracting the PR's ref is a little brittle at the moment, since the merge_group webhook payload does not include it explicitly
  • using merge-queue-base-from-pr on merge queues with the limit set greater than 1 is not recommended (will probably not work as expected).
  • did not set up a test for this since it would require merge queues enabled on this repo (if the current testing pattern were to be followed)

@janeklb
Copy link
Contributor Author

janeklb commented Jul 31, 2023

likely addresses #90 (though I haven't gone through it closely)

@andersonba
Copy link

up!

@janeklb janeklb marked this pull request as draft August 23, 2023 16:40
@janeklb janeklb marked this pull request as ready for review August 23, 2023 21:19
@janeklb janeklb changed the title feat: add support for merge_queue events feat: add support for merge queues (merge_group events) Aug 30, 2023
@meeroslav
Copy link
Collaborator

Thank you for this PR!

@meeroslav meeroslav merged commit 8419441 into nrwl:main Sep 6, 2023
6 checks passed
@janeklb janeklb deleted the jlb/merge-queue-support branch September 6, 2023 21:28
@janeklb janeklb restored the jlb/merge-queue-support branch September 6, 2023 21:28
@janeklb
Copy link
Contributor Author

janeklb commented Sep 6, 2023

Thanks @meeroslav!

I see that you've removed the option to opt-in to the special merge_group behaviour -- do you not consider there to be any risk running this against merge queue builds/branches that include multiple PRs?

@janeklb janeklb deleted the jlb/merge-queue-support branch September 6, 2023 22:03
@meeroslav
Copy link
Collaborator

So far, we have not had proper support for merge_group, but now thanks to you we do. I would rather have it on so we can polish any issues early on, rather than have it opt in a drag potential issues for months.

@janeklb
Copy link
Contributor Author

janeklb commented Sep 21, 2023

The opt in flag was for the special "branch name parsing to get the merge base" logic.

Imo generic merge group behaviour should be enabled and available (by default / always - in the same was as pull requests), but it's this extra bit I was asking about

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants