-
Notifications
You must be signed in to change notification settings - Fork 95
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
Move deny warnings to CI #205
Conversation
What is nice about this is that we deny warnings for the stable version of Rust that we support, and we ignore warnings for the nightly version, which has more changes more often. Closes #189
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.
looking at #204 and the ci failure and I think I'm wondering why the warning triggered now 😮 if we're using the same nightly binary we should have the same warning rules right?
thinking we should keep warnings on for nightly as well? Like in this case the warning is actually useful since its something we'll need to fix eventually anyways, we might as well do it as soon as its introduced?
error: use of deprecated associated function `std::sync::atomic::AtomicUsize::compare_and_swap`: Use `compare_exchange` or `compare_exchange_weak` instead
Step #4 - "external-doc-test": --> src/extensions/filters/local_rate_limit/mod.rs:138:57
Step #4 - "external-doc-test": |
Step #4 - "external-doc-test": 138 | ... refilled = available_tokens.compare_and_swap(
Step #4 - "external-doc-test": | ^^^^^^^^^^^^^^^^
Step #4 - "external-doc-test": |
Step #4 - "external-doc-test": note: the lint level is defined here
Step #4 - "external-doc-test": --> src/lib.rs:18:9
Step #4 - "external-doc-test": |
Step #4 - "external-doc-test": 18 | #![deny(warnings)]
Step #4 - "external-doc-test": | ^^^^^^^^
Step #4 - "external-doc-test": = note: `#[deny(deprecated)]` implied by `#[deny(warnings)]`
Step #4 - "external-doc-test":
Step #4 - "external-doc-test": error: use of deprecated associated function `std::sync::atomic::AtomicUsize::compare_and_swap`: Use `compare_exchange` or `compare_exchange_weak` instead
Step #4 - "external-doc-test": --> src/extensions/filters/local_rate_limit/mod.rs:169:38
Step #4 - "external-doc-test": |
Step #4 - "external-doc-test": 169 | if self.available_tokens.compare_and_swap(
Because the CI image got rebuilt (I didn't do a pull/push, I rebuilt the build script to point to the new location), we got a new nightly build -- which is more up to date.
I have no problem with warnings being visible 👍 (in fact I tested this when I updated nightly locally, and the warnings still show up when running the I'm assuming that having warnings be visible, but not cause an abort of the process is what you mean by "on" ? |
Ah that explains why it warns now make sense!
I was referring to failing the build as usual if nightly produces warnings as well but yeah I think as you mentioned it makes sense to not do so since people might have different nightlys and we don't lose much otherwise. the current fix still LGTM! |
What is nice about this is that we deny warnings for the stable version of Rust that we support, and we ignore warnings for the nightly version, which has more changes more often.
Closes #189