-
Notifications
You must be signed in to change notification settings - Fork 289
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 cargo from bors to merge-queue #1590
Conversation
|
||
[access.teams] | ||
cargo = "write" | ||
|
||
[[branch-protections]] | ||
pattern = "master" | ||
ci-checks = ["conclusion"] |
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.
ci-checks will be needed on the rust-1.*
branch protection, too. We use those for beta backports.
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.
Or...hm.. Merge queues don't support *
patterns. That's...unfortunate.
I suppose we can merge without using queues, since we almost never have more than one PR targeting a branch at once. We can assume the PR is up-to-date. Perhaps we could expose the "Require branches to be up to date before merging" setting for the beta branches?
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.
If it's needed only for cargo (and rust-lang/rust), I would just configure it manually. TIL about the *
limitation, that's annoying.
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.
Just checking, I think it would be good to also add ci-checks to the beta branch to ensure that nothing is merged with failures.
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.
Changed.
The cargo team discussed switching, and agreed we would like to try it out. I just have a minor caveat above, and then I think we should be good to go. |
e78aefb
to
769405c
Compare
repos/rust-lang/cargo.toml
Outdated
[[branch-protections]] | ||
pattern = "beta" | ||
ci-checks = ["conclusion"] |
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.
Oh, sorry, to be clear, the pattern isn't "beta". It should be the rust-1.*
pattern below. That's where we put our beta backports (it works a little differently than rust-lang/rust).
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.
Right, sorry, my bad. Fixed :)
769405c
to
6ba80a4
Compare
We should have ideally someone from the Cargo team online for performing the switch (and we also need an infra-admin). If you're willing to help, @ehuss, let me and @MarcoIeni and we can coordinate the time. |
I should be around tomorrow (Friday) if you want. |
I'm on a train for RustLab but it works for me. I'll be available in 4 hours from now for 7 hours (10.30 to 17.30 cet). EDIT: this PR LGTM. I'll approve and merge when we do the migration. |
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.
Approving and merging to switch cargo to merge queues
Accompanying PR: rust-lang/cargo#14718
Merge should be synchronized with an infra-admin switching the repo to merge queues!