Conversation
|
rust-lang/rust#57488 ... it's the 11th now so safe to try again? |
dhardy
left a comment
There was a problem hiding this comment.
There are a lot of intra_doc_link_resolution_failure warnings when building the doc for this — and yet the links do work. Should we turn these warnings off? Is it a rustdoc bug? I'd prefer not to suppress real warnings but really don't want a bunch of spurious ones polluting the output.
I didn't review everything yet — lets discuss these points first.
| //! the optimal implementations are not trivial), and this allows | ||
| //! [`ReseedingRng`] perform periodic reseeding with very low overhead. | ||
| //! `ReseedingRng` (see [`rand`](https://docs.rs/rand) crate) perform periodic | ||
| //! reseeding with very low overhead. |
There was a problem hiding this comment.
This is my second point in #690. I don't much like it — it depends on docs.rs and even so isn't a direct link — but I guess it's one option.
I would prefer such a link point to the crates.io page, possibly with a second link pointing to the API docs, or as discussed in rust-lang/docs.rs#204, be a direct link like [rand::rngs::adapter::ReseedingRng] (unfortunately I cannot get this to work).
There was a problem hiding this comment.
Ideally I would like to [rand::rngs::adapter::ReseedingRng] (or something similar) to work , but AFAIK there is currently no good way to do it except absolute links. So I think it's alright to use docs.rs links as a temporary solution, which will be easy to replace in future.
| //! [`RngCore`]: ../trait.RngCore.html | ||
| //! [`fill_bytes`]: ../trait.RngCore.html#tymethod.fill_bytes | ||
| //! [`ReseedingRng`]: ../../rand/rngs/adapter/struct.ReseedingRng.html | ||
| //! [`BlockRngCore`]: crate::block::BlockRngCore |
There was a problem hiding this comment.
So items from the crate root (RngCore) get auto-linked but not those from the same module (BlockRngCore)?
There was a problem hiding this comment.
Yes, it's a weird restriction, you can reference imported items, but not defined in the current module. It probably deserves a rustdoc issue.
| //! [`Error`]: struct.Error.html | ||
| //! [`impls`]: impls/index.html | ||
| //! [`le`]: le/index.html | ||
| //! [`rand`]: https://docs.rs/rand |
There was a problem hiding this comment.
ditto — this was previously linked to the crate page
There was a problem hiding this comment.
I think linking docs.rs is a better choice here, because most likely while browsing documentation users will want to read rand docs, and not see the crates.io page, so we will save one click for them.
dhardy
left a comment
There was a problem hiding this comment.
Okay, looks good (bar a few minor language quibbles)!
#690
Links should be checked in the generated documentation, as some errors have may slipped through. For non-dependency links I've used links to crates (grep docs.rs links) instead of links directly to items.
Also there are weird warnings for
rand_core, if documentation is generated in its folder there is no errors, but if we runcargo docin the root folder it spits several "[..] cannot be resolved, ignoring it..." errors, but those links are generated correctly...