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

Plugins deprecation: don’t suggest simply removing the attribute #65498

Merged
merged 1 commit into from
Oct 19, 2019

Conversation

SimonSapin
Copy link
Contributor

Building Servo with a recent Nightly produces:

warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See https://github.com/rust-lang/rust/issues/29597
  --> components/script/lib.rs:14:1
   |
14 | #![plugin(script_plugins)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
   |
   = note: `#[warn(deprecated)]` on by default

First, linking to #29597 is not ideal since there is pretty much no discussion there of the deprecation and what can be used instead. This PR changes the link to the deprecation PR which does have more discussion.

Second, the “remove this attribute” suggestion is rather unhelpful. Just because a feature is deprecated doesn’t mean that simply removing its use without a replacement is acceptable.

In the case of custom lint, there is no replacement available. Prefixing a message with “help:” when telling users that they’re screwed honestly feels disrespectful.

This PR also changes the message to be more factual.

Building Servo with a recent Nightly produces:

```rust
warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See rust-lang#29597
  --> components/script/lib.rs:14:1
   |
14 | #![plugin(script_plugins)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
   |
   = note: `#[warn(deprecated)]` on by default
```

First, linking to rust-lang#29597 is not ideal
since there is pretty much no discussion there of the deprecation
and what can be used instead.
This PR changes the link to the deprecation PR which does have more discussion.

Second, the “remove this attribute” suggestion is rather unhelpful.
Just because a feature is deprecated doesn’t mean that simply removing its use
without a replacement is acceptable.

In the case of custom lint, there is no replacement available.
Prefixing a message with “help:” when telling users that they’re screwed
honestly feels disrespectful.

This PR also changes the message to be more factual.
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2019
@Centril
Copy link
Contributor

Centril commented Oct 17, 2019

r? @Centril @bors r+

@bors
Copy link
Contributor

bors commented Oct 17, 2019

📌 Commit 71b0049 has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 17, 2019
@Centril
Copy link
Contributor

Centril commented Oct 17, 2019

(I didn't really try to optimize the message or anything, the goal was instead to do the absolute bare minimum of work required.)

@SimonSapin
Copy link
Contributor Author

Yeah, working on this patch showed that “remove this attribute” is merely the default for deprecated attributes when no other suggestion is specified. But the end result was still pretty jarring.

tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
Plugins deprecation: don’t suggest simply removing the attribute

Building Servo with a recent Nightly produces:

```rust
warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See rust-lang#29597
  --> components/script/lib.rs:14:1
   |
14 | #![plugin(script_plugins)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
   |
   = note: `#[warn(deprecated)]` on by default
```

First, linking to rust-lang#29597 is not ideal since there is pretty much no discussion there of the deprecation and what can be used instead. This PR changes the link to the deprecation PR which does have more discussion.

Second, the “remove this attribute” suggestion is rather unhelpful. Just because a feature is deprecated doesn’t mean that simply removing its use without a replacement is acceptable.

In the case of custom lint, there is no replacement available. Prefixing a message with “help:” when telling users that they’re screwed honestly feels disrespectful.

This PR also changes the message to be more factual.
tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
Plugins deprecation: don’t suggest simply removing the attribute

Building Servo with a recent Nightly produces:

```rust
warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See rust-lang#29597
  --> components/script/lib.rs:14:1
   |
14 | #![plugin(script_plugins)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
   |
   = note: `#[warn(deprecated)]` on by default
```

First, linking to rust-lang#29597 is not ideal since there is pretty much no discussion there of the deprecation and what can be used instead. This PR changes the link to the deprecation PR which does have more discussion.

Second, the “remove this attribute” suggestion is rather unhelpful. Just because a feature is deprecated doesn’t mean that simply removing its use without a replacement is acceptable.

In the case of custom lint, there is no replacement available. Prefixing a message with “help:” when telling users that they’re screwed honestly feels disrespectful.

This PR also changes the message to be more factual.
tmandry added a commit to tmandry/rust that referenced this pull request Oct 18, 2019
Plugins deprecation: don’t suggest simply removing the attribute

Building Servo with a recent Nightly produces:

```rust
warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See rust-lang#29597
  --> components/script/lib.rs:14:1
   |
14 | #![plugin(script_plugins)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
   |
   = note: `#[warn(deprecated)]` on by default
```

First, linking to rust-lang#29597 is not ideal since there is pretty much no discussion there of the deprecation and what can be used instead. This PR changes the link to the deprecation PR which does have more discussion.

Second, the “remove this attribute” suggestion is rather unhelpful. Just because a feature is deprecated doesn’t mean that simply removing its use without a replacement is acceptable.

In the case of custom lint, there is no replacement available. Prefixing a message with “help:” when telling users that they’re screwed honestly feels disrespectful.

This PR also changes the message to be more factual.
bors added a commit that referenced this pull request Oct 19, 2019
Rollup of 19 pull requests

Successful merges:

 - #65016 (Always inline `mem::{size_of,align_of}` in debug builds)
 - #65197 (Prepare `MutVisitor`s to handle interned projections)
 - #65201 (Disable Go and OCaml bindings when building LLVM)
 - #65334 (Add long error explanation for E0575)
 - #65364 (Collect occurrences of empty blocks for mismatched braces diagnostic)
 - #65455 (Avoid unnecessary `TokenTree` to `TokenStream` conversions)
 - #65472 (Use a sharded dep node to dep node index map)
 - #65480 (Speed up `LexicalResolve::expansion()`)
 - #65493 (Add long error explanation for E0584)
 - #65496 (properly document panics in div_euclid and rem_euclid)
 - #65498 (Plugins deprecation: don’t suggest simply removing the attribute)
 - #65508 (add option to ping llvm ice-breakers to triagebot)
 - #65511 (save-analysis: Nest tables when processing impl block definitions)
 - #65513 (reorder fmt docs for more clarity)
 - #65532 (doc: make BitSet intro more short)
 - #65535 (rustc: arena-allocate the slice in `ty::GenericsPredicate`, not the whole struct.)
 - #65540 (show up some extra info when t!() fails)
 - #65549 (Fix left/right shift typo in wrapping rotate docs)
 - #65552 (Clarify diagnostics when using `~` as a unary op)

Failed merges:

 - #65390 (Add long error explanation for E0576)
 - #65434 (Add long error explanation for E0577)
 - #65471 (Add long error explanation for E0578)

r? @ghost
@bors bors merged commit 71b0049 into rust-lang:master Oct 19, 2019
@SimonSapin SimonSapin deleted the plugin-help branch November 28, 2019 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants