-
Notifications
You must be signed in to change notification settings - Fork 892
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
feat(cli/rustup-mode): allow rustup doc
with both a flag and a topic
#4070
Conversation
8e99ab6
to
a1f70d7
Compare
a1f70d7
to
43757fb
Compare
PS: The CI failures seem irrelevant to the changes being made in this PR. |
This issue has been resolved in rust-lang/rust#133532. |
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.
LGTM! (I assume you'll rebase this to ditch the merge commit?)
4f46570
to
19a006b
Compare
19a006b
to
90a6e35
Compare
90a6e35
to
637ec57
Compare
@djc Do you think we can merge this change now, so that it can join the others in the upcoming beta? 🙏 |
I already approved it, so feel free to merge unless there's something in particular you want me to look at. |
@djc Thanks! There's probably #4070 (comment) although I don't think it should be blocking it being merged. Let's do the merge first and see. |
This PR is my stab at the concern in #4069.
However, I think
rustup
should not magically detect and acknowledge lints, so I've made a more neutral solution that might apply to more topics:Concerns
Should we add test cases for this PR, and how may we add them?Added--path
tests to ensure that we can get the doc paths right.