-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 CI from bors to merge queue #13587
base: master
Are you sure you want to change the base?
Conversation
295ed0b
to
c7ec86e
Compare
@@ -1,17 +1,6 @@ | |||
name: Clippy Test | |||
|
|||
on: | |||
push: |
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.
This was there, so that people also get to run the Clippy CI in their forks and don't have to open a PR.
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.
I see. That's a bit problematic with the merge queue, because it creates separate branches for each merge queue test. But we don't want to run CI twice on these. I'm not sure if there's a single prefix that we could use here to ignore these.
We could potentially just use a push:
hook and then somehow skip the workflow if the repository is rust-lang/rust-clippy 🤔
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.
We could potentially just use a
push:
hook and then somehow skip the workflow if the repository is rust-lang/rust-clippy
That sounds a bit too hacky. TBH I never looked at CI results in my fork. Not sure anymore who requested that. We could do the scream test and just remove it and see if anyone complains 😈
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.
I agree. Usually the only interaction of CI in forks that I see is people being annoyed by an e-mail flurry spam about failing CI workflows after forking something.
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.
I have a PR on my fork where I do all kinds of experiments so I can see the lintcheck results (the lintcheck workflow only runs on PRs), and I believe @xFrednet
mentioned on Zulip they do the same IIRC. But seeing how the lintcheck.yml file is unchanged, I assume this is unaffected and will still work?
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.
Indeed, lintcheck is untouched by this PR.
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.
Also: If you open a PR in a fork (instead of just pushing a branch) all checks will run anyway. This is more about not having to open a PR in your fork to get the clippy workflow result.
Not sure if I understand the CI error: https://github.com/rust-lang/rust-clippy/actions/runs/11513171989/job/32049393605#step:6:39. Does it actually check that the link returns 200? In that case, we'll need to update the badge only after merging this. |
Yes, it checks that all links in our markdown files actually lead to something (this check is actually copied from the Rust repo, so that we don't run into problems during sync). |
63790ff
to
3338611
Compare
Ok, I removed the badge URL changes, we can fix them up after this is merged. |
CI failure looks spurious. |
I restarted the run. It was spurious. |
Context: https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Reducing.20the.20amount.20of.20repos.20that.20use.20bors.2Fhomu
This should be merged with cooperation from an infra-admin.
Opening as a draft, because the CI on this repo is.. interesting :) So I want to make sure that the changes make sense. I renamed
clippy_bors.yml
toclippy.yml
andclippy.yml
toclippy_pr.yml
, which makes the diff a bit.. tricky.