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

Use Merge Queue instead of requiring PRs to be up to date #737

Open
XAMPPRocky opened this issue Apr 20, 2023 · 8 comments
Open

Use Merge Queue instead of requiring PRs to be up to date #737

XAMPPRocky opened this issue Apr 20, 2023 · 8 comments
Labels
area/build-tools Development tooling. area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc kind/cleanup Refactoring code, fixing up documentation, etc

Comments

@XAMPPRocky
Copy link
Collaborator

GitHub has a new merge queue feature, this would replace the need to have the requirement that all PRs are up to date with main before they can be merged, as the point of the merge queue is to handle that automatically for you to prevent merge skew conflicts. This would save a lot of time, as if I open a lot of different pull requests, and even if they're all approved it could an additional hours of manually clicking "update" on each the PR as another gets merged before everything gets merged.

https://github.blog/changelog/2023-02-08-pull-request-merge-queue-public-beta/

@XAMPPRocky XAMPPRocky changed the title Use Merge Queue instead of requiring Pas to be up to date Use Merge Queue instead of requiring PRs to be up to date Apr 20, 2023
@XAMPPRocky XAMPPRocky added area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. labels Apr 20, 2023
@markmandel
Copy link
Contributor

markmandel commented Apr 20, 2023

Merge queue is really nice - it doesn't tend to work well with our CLA bot at the moment.

I did setup on Agones adRise/update-pr-branch (https://github.com/googleforgames/agones/blob/main/.github/workflows/pr_update.yml) and that's been working really well.

@XAMPPRocky
Copy link
Collaborator Author

It's now generally available. Would we be able to try it?

@markmandel
Copy link
Contributor

I just did some hunting internally, and I think it does work now with our CLA bot. Let's take it for a spin!

There's some things I'll need to do with our CI I think, but they should be pretty easy:
https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue#triggering-merge-group-checks-with-third-party-ci-providers

@markmandel
Copy link
Contributor

I've turned on merge queues, and I'm fairly sure I have Cloud Build CI setup (assuming my regex is good), so we can take this for a spin and see how it goes.

@markmandel
Copy link
Contributor

markmandel commented Jul 21, 2023

So here lies a tricky problem I'm not 100% sure how to solve.

You can see it here:
image

Status: CI (quilkin) only runs on GitHub PR (current not required)
Status: CI-merge-queue (quilkin) only runs on merge queue branch creation. (required).

#764 only merge because I edited the required status checks during a merge queue process.

I don't think I can make the two the same.

Also not sure How I'm allowed to "merge when ready" when there aren't enough approval on a PR.

See #712 for example. Looks like we could click "merge when ready", even though there are no approvals.

Also noting you can't reword using merge queues either.

I think this above is going to be a dealbreak for us. I'll leave it alone for now, see how it goes, and see if we see another way through.

So I may suggest we also adopt the workflow in #737 (comment) with automatically updates PRs that are set to automerge on passing, so that people don't have to click the button to update either.

It's been working really well for the Agones project.

@markmandel
Copy link
Contributor

Also not sure How I'm allowed to "merge when ready" when there aren't enough approval on a PR.

Chatted with GitHub friends - so the PR won't be added to the merge queue unless it's approved. Not well documented, but that's actually kinda useful as you can pre-bank PRs that are awaiting approval.

@markmandel
Copy link
Contributor

I think this above is going to be a dealbreak for us. I'll leave it alone for now, see how it goes, and see if we see another way through.

Just playing with this some more - not 100% sure on this. The current branch protection requirements might be fine. We may have have to try it and see. So leaving alone as it is.

@markmandel
Copy link
Contributor

Going to go bug some Cloud Build PMs to see what's possible 😄

In the meantime, I'll get https://github.com/googleforgames/agones/blob/main/.github/workflows/pr_update.yml going, so we can automate PR's that are set to auto-merge.

Not as nice as merge queues, but at our rate of throughput, should be good enough for now.

markmandel added a commit to markmandel/quilkin that referenced this issue Aug 2, 2023
GitHub action that will sequentially update to `main` PRs that pass all
their checks and are set to "auto-merge".

Not quite as nice as a merge queue, but at least means you don't have to
manually manage a queue of approved PRs.

Work on googleforgames#737
XAMPPRocky pushed a commit that referenced this issue Aug 2, 2023
GitHub action that will sequentially update to `main` PRs that pass all
their checks and are set to "auto-merge".

Not quite as nice as a merge queue, but at least means you don't have to
manually manage a queue of approved PRs.

Work on #737
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build-tools Development tooling. area/user-experience Pertaining to developers trying to use Quilkin, e.g. cli interface, configuration, etc kind/cleanup Refactoring code, fixing up documentation, etc
Projects
None yet
Development

No branches or pull requests

2 participants