Open epage opened 5 years ago
I'm personally strongly against making warnings errors. It means that, for example, a minor version release for a dependency that deprecates a method can cause your build to fail, even though the version bump was semver compatible. If authors specifically want no warnings, they should opt into that with
#![deny(warnings)]
in their src/lib.rs
(or equivalent)
I'm strongly in favor of turning warnings into errors :)
clap
broke people.#![deny(warnings)]
is an anti-pattern that causes problems.
master
.Warnings as errors breaks safe semver updates, which is a deal-breaker for me (the example of dependency deprecations being one example, macros as you mention being another).
I'm not sure I know about the flag you mention so that new stable
releases don't break your build with -D warnings
?
I completely agree with you that your code should compile without warnings. The problem is that new warnings can appear without your code changing. For example, as discussed in another issue, this could absolutely cause a new contributor to see red CI unrelated to their current changes, such as if Cargo.lock
is not checked in and CI uses a later minor version of a dependency that deprecates a method.
This all said, I think it might be okay to include a deny_warnings: true
parameter to the cargo-check
template (and thus also to stages
) for people to opt in to it. I definitely think it should be false
by default though.
So I've never run into a dependency deprecating something, only stdlib which has been handled by warning on MSRV only.
A way of handling this is by instead doing
RUSTFLAGS="-D warnings -A deprecated"
Outside of macros, deprecated
is the only way I know of for a dependency to break you via warnings, so I think this should be sufficient.
fwiw, #33 mostly addresses this by running a stage that treats all warnings as errors (including deprecations), but treat them as allowed failures (so they'll show up as yellow CI). I think that's a reasonable compromise?
Doing a
cargo check
withRUSTFLAGS="-D warnings"
will fail the build on warnings.The problems is new warnings can be added and break the build, so like clippy in #18, we should use a pinned version of rustc.