Skip to content
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.

Sign up for GitHub

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

Fail the build on rustc warnings #95

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fail the build on rustc warnings #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
896ac34
crane: fail build on warnings
johanot Oct 19, 2024
197d3c5
issuer/resolvererror: allow enum tuple field to never appear to be read
johanot Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
# upstream ideally) so this interface might be brittle, but avoids
# accidentally passing a detached nixpkgs from its flake (or its follows)
# on to consumers.
craneLib = crane.mkLib final;
craneLib = (crane.mkLib final).overrideScope (_: scopePrev: {
mkCargoDerivation = args: scopePrev.mkCargoDerivation ({
RUSTFLAGS = "-D warnings";
} // args);
});
Comment on lines +29 to +33
Copy link
Member

@cafkafk cafkafk Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if this was done elsewhere, as it's rather aggressive to deny warnings (such as e.g. unused variables not being prefixed with underscores).

I think if the goal is to increase correctness of the codebase, adding a clippy flake check with all the flags, and then putting it in CI makes much more sense, there are examples of such checks in the crane repo in the examples folder that could essentially be copied.

That way we don't break fast iteration speed of "compiler driven development" but instead move the validation of pedantics to the finalization of a contribution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably just me that primarily uses cargo (rather than nix build) for compiler-driven-dev (shrug). I'm fine with moving it to a flake check as long as it fails the GH pipeline protecting main :) Will look at it as soon as possible.

};
in {
packages.${system}.${pname} = pkgs.${pname};
Expand Down
5 changes: 4 additions & 1 deletion src/issuer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ impl std::convert::From<common::SpecError> for IssuerError {
#[derive(Debug)]
enum ResolverError<'l> {
SystemResolveConf,
NoIpsForResolversFound(&'l String),
NoIpsForResolversFound(
#[allow(dead_code)]
&'l String
),
Other
}

Expand Down
Loading