-
Notifications
You must be signed in to change notification settings - Fork 292
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
Blog post on removing the Clippy plugin interface #435
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
r? @Manishearth |
layout: post | ||
title: "Clippy is removing its plugin interface" | ||
author: Philipp Krones | ||
description: "Now that compiler plugins are deprecated, Clippy is also removing its plugin interface" |
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.
description: "Now that compiler plugins are deprecated, Clippy is also removing its plugin interface" | |
description: "Clippy is removing its deprecated plugin interface" |
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.
Hm that is practically the title then.
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
cc @Centril for wording |
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
Thanks for the reviews! |
I wonder if this would be better suited to the main rust blog? It seems like the audience is perhaps closer to "all Rust users" than "all Rust contributors". |
3. Replace every occurrence of `feature = "clippy"` with `feature = | ||
"cargo-clippy"`. The `cargo-clippy` feature is automatically enabled when | ||
running `cargo clippy`. |
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.
I think this would typically be the wrong guidance. People used feature = "clippy"
because the canonical lint suppression used to be:
#[cfg(feature = "clippy", allow(name_of_lint))]
The correct migration would be to a tool attr:
#[allow(clippy::name_of_lint)]
There might be remaining use cases for #[cfg(feature = "cargo-clippy")]
but it would be extremely unusual, right?
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.
This is already handled by rustc. Running cargo clippy
on
#![cfg_attr(feature = "cargo-clippy", deny(if_not_else))]
// code ...
emits
warning: lint name `if_not_else` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore
--> src/main.rs:1:44
|
1 | #![cfg_attr(feature = "cargo-clippy", deny(if_not_else))]
| ^^^^^^^^^^^ help: change it to: `clippy::if_not_else`
|
= note: `#[warn(renamed_and_removed_lints)]` on by default
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 I wanted to write a clippy lint, that detects cfg_attr(cargo-clippy)
and suggests to remove this, but never got to finish this lint.
https://github.com/flip1995/rust-clippy/commits/cfg_attr_lint
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
posts/inside-rust/2019-10-24-Clippy-removes-plugin-interface.md
Outdated
Show resolved
Hide resolved
I moved it to the main rust blog. If anyone thinks it should stay in inside-rust, I can move it back anytime. |
cc @nikomatsakis @Centril @Manishearth We'd like to merge rust-lang/rust-clippy#4714, but would like to publish this blog post first. Are there any more comments to this post? Note that it now aimed at the main rust blog. Should I adapt the date in the file name before merging? |
Hmm. I know I suggested the main blog -- but I'm second guessing that. My main concern is that -- reading this post as a non-clippy user -- I am a bit confused as to its significance. My assumption is that you are removing a "little used" way to integrating clippy, and that this won't affect most users. But the post doesn't make that context clear. I worry that casual readers will be confused into thinking "what's this? clippy is being removed?" I suppose this isn't really a question as to which blog is appropriate but rather just feedback on the post -- it'd be good to add a few lines putting this change into some greater context. For example: "We don't expect this change to effect most users. It only effects you if you are using clippy in this particular way, which is not the common path." (obviously with more details). Of course, if I am wrong, and this actually does effect a lot of users, ok! |
@nikomatsakis right, that's why we wanted it on the inside blog, it doesn't affect most folks but we should write about it somewhere and link to it on the forums. |
1a46128
to
03e0564
Compare
@nikomatsakis I added the note and moved it back to the inside rust blog. |
OK, sorry for the confusion! going to merge. |
Clippy is removing its plugin interface rust-lang/rust-clippy#4714. Even though the plugin interface has been deprecated for a long time, we want to provide some instructions on how to migrate from the plugin interface.
Rendered
cc @phansch @Manishearth