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.

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

Add blog post about deprecating feature = "cargo-clippy" #1253

Merged

Conversation

flip1995
Copy link
Member

I'm not sure, if this blog post should go into the internals Rust blog or the main blog. I put it in the main blog for now as it might affect a bunch of users, but I'm happy to move it to internals.

cc @rust-lang/clippy @Urgau

Rendered

Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

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

Great. Looks good to me.

@flip1995
Copy link
Member Author

Thanks for the reviews!

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

Typo (meow)

@flip1995 flip1995 force-pushed the clippy-feature-deprecation branch from c88ed9e to 7a505c5 Compare February 19, 2024 09:51
@flip1995
Copy link
Member Author

@Mark-Simulacrum Not sure who's responsible for merging blog PRs, so pinging you, as I think that you might know: This change will go into nightly with the sync on Thursday (so on Friday), so we'd like to merge this blog post before then. Last open question is: Should this go into Rust internals or the main Rust blog?

@flip1995 flip1995 force-pushed the clippy-feature-deprecation branch from 7a505c5 to 771d6f3 Compare February 28, 2024 09:21
@flip1995
Copy link
Member Author

I updated the date in the file name to today. I'd really like to get this merged today, as people will start seeing the deprecation warnings triggering today in nightly: Playground

@Mark-Simulacrum Mark-Simulacrum merged commit 65078d5 into rust-lang:master Feb 28, 2024
3 checks passed
@Mark-Simulacrum
Copy link
Member

A DM on Zulip is probably the most reliable way to get immediate action (i.e. please merge this).

@flip1995 flip1995 deleted the clippy-feature-deprecation branch February 28, 2024 13:05
@danielparks
Copy link

Hello, this change affects me (I actually wrote the documentation linked in the post). Could you clarify when this takes effect? Currently it reads like this applies now, but:

  • ❌ Running cargo clippy on code that uses feature = "cargo-clippy" does not complain.
  • ❌ Running cargo +nightly clippy on code that uses feature = "cargo-clippy" does not complain.
  • not(cfg(clippy)) does not work on stable clippy — clippy ignores the cfg and lints the code.
  • not(cfg(clippy)) does work on nightly clippy — clippy ignores the code.

(Naturally these are on latest stable and nightly as of a few minutes ago.)

It would be very helpful to know when I can expect these things to change.

It would not be such a big deal, but I’m having trouble figuring out which issue I should be tracking for stabilization of these changes.

@flip1995
Copy link
Member Author

Oh, I forgot to mention that this currently only works on nightly, and will only be in stable 1.78. I will update the blog post to clarify this. Sorry for the confusion.

@flip1995
Copy link
Member Author

  • Running cargo +nightly clippy on code that uses feature = "cargo-clippy" does not complain.

This is surprising. Maybe your nightly is not up-to-date? I tested this on play.rust-lang-org before merging the blog post.

#1264 Here's the update, sorry again for the confusion!

@danielparks
Copy link

Thanks!

Running cargo +nightly clippy on code that uses feature = "cargo-clippy" does not complain.

This is surprising. Maybe your nightly is not up-to-date? I tested this on play.rust-lang-org before merging the blog post.

Looks like this has something to do with my code. Clippy properly complains when I tried a minimal test case.

I generate the offensive code in build.rs inside a test crate, so it’s not exactly a common situation. Clippy catches other lints in the generated code though, so I think there may be some sort of bug. I’ll look into it further and raise a new issue once I know more, or discover that I’m doing something silly (sillier than generating 48,000 line match statements, anyway).

Sorry for the confusion! :)

@danielparks
Copy link

danielparks commented Mar 1, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants