-
Notifications
You must be signed in to change notification settings - Fork 633
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
Attempt to fix all documentation warnings from rustdoc. #1574
Conversation
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.
Thanks for working on this. I left comments to solve some TODO. And the cause of some errors was that I wrote the wrong link 😅.
Actually, intertwined with the hundreds of warnings from rand, there is another bunch from futures, so I will continue my quest |
So, a status report:
There is another bunch of warnings left, but as long as there is no clear solution for cross crate links, I don't see how to continue this:
|
(FYI, I opened #1578 to avoid all doc errors from rand.) |
I think it's difficult to fix all these warnings at this point. |
Some updates and reports:
#1574 (comment) Also, in this PR, I think that it is Ok to ignore the problem that the link does not work in the cross-crate (because as far as I know the main purpose of this PR is to fix warnings). |
ok! that all sounds like good news. I had to work on some other project today and yesterday, but I'll be back soon to finish off the work I started.
|
I attempted to fix all documentation warnings in futures-rs. All of these refer to failing cross- references. Note that it's hard to verify as there is still alot of warnings from dependencies like rand and from what I suspect to be std, but it's unclear, since rustdoc doesn't mark the origin of the warnings. See: rust-lang/rust#59367 and rust-lang/rust#55907 The fixes here contain several TODO's, for two main reasons: - false positive warnings from rustdoc, where rustdoc generates a correct link but still issues a warning - places where I have been obliged to put a link to an html file because I didn't manage to make rustdoc generate a correct link from a path. It would be nice if people verified that commits don't throw warnings before merging, even in rustdoc. Especially so because rustdoc does not hide warnings in dependencies right now, even when called with `--no-deps`. That means that any warnings thrown by futures-rs will bother every single dev that has a (indirect) dependency on futures-rs and runs rustdoc.
TODO: note that paths like futures_core::task::Context actually get redirected to core::task::Context in the redered docs. Is this desirable? If so, should we change the paths here? Also, there is cross crate links in here. They are not going to work anytime soon. Do we put https links in here? to here: https://rust-lang-nursery.github.io/futures-api-docs? The problem is these have a version hardcoded in the url: 0.3.0-alpha.15 We could link to docs.rs, but currently that says: docs.rs failed to build futures-preview-0.3.0-alpha.15 -> ok the reason seems to be that they are on 2019-04-17 which does still have futures-api unstable feature, so that should get solved.
…o clear solution... when the build on docs.rs passes again, we could decide to link to docs.rs, until then I have no solution.
02a1d66
to
5f26475
Compare
those commits are just rebase on latest master, fixing more warnings now |
but it will fail to generate correct links when rustdoc is run at the futures-rs level. Eg, the re-exports fail: futures::channel::oneshot::Receiver
ok, I think that's if for now. There is a lot of warnings from std, and the ones in futures-channel I don't get right. They work fine when running rustdoc in the futures-channel crate, but not when running it for all futures-rs. I did find cross crate links working, but only to the original, not to re-exports. Hence links to |
Thanks so much for the help! :) |
@cramertj Thank you for all the work on async and other rust. If I knew the commits were not going to be squashed, I would have made a bit more of a clean history, but then again, it probably doesn't change much in real life! |
No worries-- if I were bothered about it I would've squashed them myself. :) |
I attempted to fix all documentation warnings in futures-rs. All of these refer to failing cross-
references. Note that it's hard to verify as there is still alot of warnings from dependencies
like rand and from what I suspect to be std, but it's unclear, since rustdoc doesn't mark the
origin of the warnings. See: rust-lang/rust#59367 and rust-lang/rust#55907
The fixes here contain several TODO's, for two main reasons:
a warning
rustdoc generate a correct link from a path.
It would be nice if people verified that commits don't throw warnings before merging,
even in rustdoc. Especially so because rustdoc does not hide warnings in dependencies
right now, even when called with
--no-deps
. That means that any warnings thrown byfutures-rs will bother every single dev that has a (indirect) dependency on futures-rs
and runs rustdoc.