Add warning checks to the complete template

[epage]

Doing a cargo check with RUSTFLAGS="-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.

[jonhoo]

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)

[epage]

I'm strongly in favor of turning warnings into errors :)

• Dependency warnings do not break your build.
  • However, Macros from dependencies can. There was a time that clap broke people.
• Deprecation warnings from using a new rustc will also not break your build unless you opt-in to it that warning doing so
• Warnings are just as valuable as clippy. If we have one, we should have the other.
• #![deny(warnings)] is an anti-pattern that causes problems.
  • You cannot pin your Rust version causing you to be subject to breakage as new warnings are added.
  • It also impairs development iteration where warnings can be minor while you don't want them in master.

[jonhoo]

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.

[epage]

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.

[jonhoo]

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?