-
Notifications
You must be signed in to change notification settings - Fork 337
Use Nightly + Cranelift for dev, only fail on warnings in CI #4388
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
Conversation
#[serde(default = "HashMap::new")] | ||
pub file_types: HashMap<String, Option<FileType>>, | ||
} | ||
|
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.
Idk how this wasn't triggering a dead code warning before, but apparently this is unused. On standard codegen this doesn't emit a warning. On cranelift this does.
Self::StaleWhileRevalidateSkipOffline | ||
} | ||
} | ||
|
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 don't know how this wasn't caught on main CI.
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.
That is most likely explained due to the usage of different Rust versions. Nightly's Clippy is ahead of stable when it comes to new lints (and regressions).
3f2ad4e
to
137f338
Compare
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
PR #3960 reverted the Cranelift usage introduced in #4388 due to its codegen not being up to standard when compiling some pieces of code under some platforms. However, it didn't revert the switch to a nightly Rust toolchain, which is now unnecessary, and produces unnecessary drift between what's declared in the `rust-toolchain.toml` and the Docker image manifests, causing inefficiencies. These changes bring back the usage of stable Rust for the time being to correct those inefficiencies.
PR #3960 reverted the Cranelift usage introduced in #4388 due to its codegen not being up to standard when compiling some pieces of code under some platforms. However, it didn't revert the switch to a nightly Rust toolchain, which is now unnecessary, and produces unnecessary drift between what's declared in the `rust-toolchain.toml` and the Docker image manifests, causing inefficiencies. These changes bring back the usage of stable Rust for the time being to correct those inefficiencies.
Cranelift has much faster incremental recompiles, which helps when iterating on labrinth features.
Warnings are currently denied in the root Cargo.toml, which is good for rigorous CI but is annoying when quickly prototyping features and you don't mind some dead code or unused imports. This PR keeps warnings as warnings, but passes
-Dwarnings
in the CI test run to fail on warnings there.