-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
don't warn about a missing change-id in CI #130356
Conversation
fixes 130352
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @albertlarsan68 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Why this doesn't work already? Lines 50 to 53 in e7386b3
Directly ignoring the change tracker at the bootstrap level could have negative effects for downstream CIs. |
as a downstream CI: I am also ignoring this in my CI because I do not care about all the notifications because they are never relevant to me. |
if something happens that is important, the CI will probably break. until then, the 9 million tiny changes that only matter to interactive users are not very relevant. it doesn't matter if my eyes glaze over when I see the message or if I actively silence it: the same effect of "I ain't readin' all that" takes place. |
Update on this: I initially thought this PR was ignoring the change messages, but it’s actually just skipping the help message for the missing field. That makes sense. I have just created an alternative PR that skips the help message for everyone by default unless running in verbose mode. |
thank you! personally happy to see whatever variation on this people like better land. |
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.
LGTM, sorry for the delay!
@bors r+ rollup |
Did you check #130368? If so, I will close that one. |
I think that those are quite orthogonal |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#130356 (don't warn about a missing change-id in CI) - rust-lang#130900 (Do not output () on empty description) - rust-lang#131066 (Add the Chinese translation entry to the RustByExample build process) - rust-lang#131067 (Fix std_detect links) - rust-lang#131644 (Clean up some Miri things in `sys/windows`) - rust-lang#131646 (sys/unix: add comments for some Miri fallbacks) - rust-lang#131653 (Remove const trait bound modifier hack) - rust-lang#131659 (enable `download_ci_llvm` test) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#130356 (don't warn about a missing change-id in CI) - rust-lang#130900 (Do not output () on empty description) - rust-lang#131066 (Add the Chinese translation entry to the RustByExample build process) - rust-lang#131067 (Fix std_detect links) - rust-lang#131644 (Clean up some Miri things in `sys/windows`) - rust-lang#131646 (sys/unix: add comments for some Miri fallbacks) - rust-lang#131653 (Remove const trait bound modifier hack) - rust-lang#131659 (enable `download_ci_llvm` test) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130356 - lolbinarycat:ci-no-change-id, r=albertlarsan68 don't warn about a missing change-id in CI fixes rust-lang#130352
…Kobzol remove `change-id` from CI script It's not necessary to set `change-id` for CI since rust-lang#130356.
Rollup merge of rust-lang#132130 - onur-ozkan:remove-ci-change-id, r=Kobzol remove `change-id` from CI script It's not necessary to set `change-id` for CI since rust-lang#130356.
fixes #130352