New lint and cargotest

sanxiyn · 2016-04-25T11:03:40.379Z

Warn unused trait imports, rebased

7 commits changed 60 files with 193 additions and 83 deletions.

I improved a lint to warn unused trait imports. This failed to merge, because the lint triggered for unused trait imports in cargo as designed, which causes cargotest to fail because of #[deny(unused)]. As far as I can tell there is no way to tell cargo to pass --cap-lints to rustc. So I am not sure how to proceed.

As is, this means new lint can’t be merged until it is fixed in all cargotest crates, as well as existing requirement that it is fixed in all of rustc and rustdoc.

brson · 2016-04-26T18:43:20.651Z

This might sound insensitive, but this is exactly why cargotest exists! If this feature is important you’ll need to convince reviewers that it’s worth breaking cargo for, then fix cargo and update cargotest. Sorry not sorry

oli-obk · 2016-04-27T14:38:24.689Z

Improving lints is not considered “breaking” as per rfc1193, so technically the fact that cargotest has #[deny(...)] attributes is the issue.

sanxiyn · 2016-05-03T10:07:09.284Z

Okay, changing the requirement from “fix all of rustc and rustdoc” to “fix all of rustc and rustdoc and cargo” is not that a big deal. Still, it is a noticable change, because Cargo is in the different repository.

But cargotest includes projects other than Cargo, which is just Iron at the moment. As I understand, more will be added in the future. Is the intention that landing new lint should be blocked on fixing Iron and additional crates to be determined in the future?

oli-obk · 2016-05-03T11:50:40.835Z

I think crates in the cargotest list should not have deny or forbid attributes. Instead their CI should set that flag through RUSTFLAGS or similar

sanxiyn · 2016-05-03T12:17:30.104Z

I think crates in the cargotest list should not have deny or forbid attributes. Instead their CI should set that flag through RUSTFLAGS or similar.

I am not sure how breaking CI is any better than breaking build. @alexcrichton expressed the same opinion at https://github.com/rust-lang/cargo/pull/2622, which also suggested “deny in CI”.

brson · 2016-05-06T17:08:53.914Z

sanxiyn:

Is the intention that landing new lint should be blocked on fixing Iron and additional crates to be determined in the future?

It’s mine yeah. Adding crates to cargotest shouldn’t be done lightly.

system · 2019-03-25T08:26:06.727Z

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.