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

Switch to GH merge queue? #3882

Closed
RalfJung opened this issue Sep 13, 2024 · 8 comments
Closed

Switch to GH merge queue? #3882

RalfJung opened this issue Sep 13, 2024 · 8 comments
Labels
A-tests Area: affects our test suite or CI C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out

Comments

@RalfJung
Copy link
Member

The idea of switching from bors to GH merge queues has come up multiple times. "Old" bors often has trouble (though in the recent past it worked just fine), and IIRC, t-infra recommends that most smaller projects switch to merge queues rather than "new" bors. This also has the advantage of working around rust-lang/rust#101907 since we'd no longer have bors commits as part of the sync.

The main downside is increased latency: github waits until PR CI passes before even putting the PR in the queue, so one can't just push + r+ immediately, one has to wait for two times the CI time until the PR actually lands. However, landing PRs is rarely latency-sensitive so I don't think this is a big enough problem.

@rust-lang/miri what do you think?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 13, 2024

Can we set PR CI to make all CI runs "optional", so their failure is irrelevant. That may allow the merge queue addition, we just need to ensure that the merge queue actually requires the CI runs to pass

But yea, seems fine to me

@RalfJung
Copy link
Member Author

Can we set PR CI to make all CI runs "optional", so their failure is irrelevant. That may allow the merge queue addition, we just need to ensure that the merge queue actually requires the CI runs to pass

That would probably lead to confusing UI where there are green checkmarks even though CI failed...

@oli-obk
Copy link
Contributor

oli-obk commented Sep 13, 2024

No, you can set CI runs as not blocking the merge button from becoming green. Even if the CI run shows up red. The only question is if we can make the merge queue have different CI rules than the PR CI

@RalfJung
Copy link
Member Author

Oh, you mean changing what is set in "Require status checks to pass before merging"... I think that's always the same. But we could make the success job actually be different depending on whether it is a PR CI run or a merge queue CI run, and make it succeed immediately in PR CI, maybe that would make it enqueue things immediately.

But it's not worth it IMO. Our CI times are <30min, so landing takes <1h, which seems entirely fine.

@RalfJung
Copy link
Member Author

@saethlin what do you think?

@RalfJung RalfJung added C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out A-tests Area: affects our test suite or CI labels Sep 17, 2024
@saethlin
Copy link
Member

saethlin commented Sep 17, 2024

The runtime seems acceptable. I've been getting a lot of practice working with merge queues at my day job, so I'm more confident switching than I was the first time you mentioned this topic.

@Kobzol
Copy link
Contributor

Kobzol commented Oct 21, 2024

Created PRs for the switch: #3981, rust-lang/team#1583

@RalfJung
Copy link
Member Author

Fixed by #3981

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tests Area: affects our test suite or CI C-proposal Category: a proposal for something we might want to do, or maybe not; details still being worked out
Projects
None yet
Development

No branches or pull requests

4 participants