-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Deprecate several flags in rustdoc #44138
Deprecate several flags in rustdoc #44138
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
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.
r=me. Probs should use the colorful one over eprintln
src/librustdoc/lib.rs
Outdated
matches.opt_present("passes") { | ||
eprintln!("WARNING: this flag is considered deprecated, please see https://github.com/rust-lang/rust/issues/44136") | ||
} | ||
} |
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.
Nl at eof
What's "the colorful one"? |
7497e31
to
36f52d9
Compare
nvm, that's only available from within rustc and nontrivial to use without a |
36f52d9
to
4cd5314
Compare
@bors: r=Manishearth |
📌 Commit 4cd5314 has been approved by |
@bors rollup |
…r=Manishearth Deprecate several flags in rustdoc Part of rust-lang#44136 cc @rust-lang/dev-tools @rust-lang/docs This is a very basic PR to start deprecating some flags; `rustdoc` doesn't really have fancy output options like `rustc` does, so I went with `eprintln!`. Happy to change it if people feel that's not appropriate. Also, I have no idea if we can or should write tests here, so I didn't try. If someone feels strongly about it, then let's do it, but given that the only outcome here is a side effect...
…r=Manishearth Deprecate several flags in rustdoc Part of rust-lang#44136 cc @rust-lang/dev-tools @rust-lang/docs This is a very basic PR to start deprecating some flags; `rustdoc` doesn't really have fancy output options like `rustc` does, so I went with `eprintln!`. Happy to change it if people feel that's not appropriate. Also, I have no idea if we can or should write tests here, so I didn't try. If someone feels strongly about it, then let's do it, but given that the only outcome here is a side effect...
Hm right now this just prints out "WARNING: this flag is considered deprecated", could it also print the flag as well? |
In rust-lang/rust#44138 we are adding deprecations, and so it breaks these tests.
e85918f
to
6438b9b
Compare
relax rustdoc tests This was asserting on the output directly, rather than just what it contains. In rust-lang/rust#44138 we are adding deprecations, and so it breaks these tests.
Great, so this works with my Cargo PR. @alexcrichton , what's the process for updating Cargo these days? It looks like it depends on a released version, I'm not sure what the protocol is for that.
Also happy to do that as well, alongside dropping the testing changes to the submodules. |
Oh you can just update the cargo submodule whenever on the master branch of rust, no worries! |
6438b9b
to
377907c
Compare
🤘 |
@alexcrichton https://travis-ci.org/rust-lang/rust/jobs/270119195#L772 what's the right way to resolve this? change it to 0.23? |
Probably rebase on top of #44154 and wait for that to land I think? |
Okay well, sorry rfcbot i'm done fighting with you :) ping the @rust-lang/dev-tools team for signoffs; here's some manual ticky boxes for you:
Primary sign-off here is " |
LGTM @klabnikatron reviewed |
@steveklabnik loos like the last remaining box here is @nrc whom I believe is on parental leave right now, although I doubt he'd object to this :) |
We looked at this during dev-tools triage. As @alexcrichton says, I think this is good to go, @steveklabnik! |
Ping again @steveklabnik 🙂. I think the teams' decisions are "it is good to go". The remaining item is to get it tested, right? |
Sorry for the delay here, will get on this tomorrow! |
Part of rust-lang#44136 Upgrades cargo due to rust-lang/cargo#4451
e5eafce
to
0b31319
Compare
0b31319
to
045ce18
Compare
Okay; we don't have a great way to test the output here, but on @QuietMisdreavus 's suggestion, I've changed a few of the rustdoc tests to use the new flag. That gives us about as much coverage as the old flags did. |
*drumroll* @bors r+ |
📌 Commit 045ce18 has been approved by |
…r=QuietMisdreavus Deprecate several flags in rustdoc Part of rust-lang#44136 cc @rust-lang/dev-tools @rust-lang/docs This is a very basic PR to start deprecating some flags; `rustdoc` doesn't really have fancy output options like `rustc` does, so I went with `eprintln!`. Happy to change it if people feel that's not appropriate. Also, I have no idea if we can or should write tests here, so I didn't try. If someone feels strongly about it, then let's do it, but given that the only outcome here is a side effect...
…=GuillaumeGomez rustdoc: deprecate `#![doc(passes, plugins, no_default_passes)]` Closes rust-lang#48164 Blocked on rust-lang#50541 - this includes those changes, which were necessary to create the UI test cc rust-lang#44136 Turns out, there were special attributes to mess with rustdoc passes and plugins! Who knew! Since we deprecated the CLI flags for this functionality, it makes sense that we do the same for the attributes. This PR also introduces a `#![doc(document_private_items)]` attribute, to match the `--document-private-items` flag introduced in rust-lang#44138 when the passes/plugins flags were deprecated. I haven't done a search to see whether these attributes are being used at all, but if the flags were any indication, i don't expect to see any users of these.
Part of #44136
cc @rust-lang/dev-tools @rust-lang/docs
This is a very basic PR to start deprecating some flags;
rustdoc
doesn't really have fancy output options likerustc
does, so I went witheprintln!
. Happy to change it if people feel that's not appropriate.Also, I have no idea if we can or should write tests here, so I didn't try. If someone feels strongly about it, then let's do it, but given that the only outcome here is a side effect...