rust-cli / env_logger

A logging implementation for `log` which is configured via an environment variable.
https://docs.rs/env_logger
Apache License 2.0
803 stars 125 forks source link

Usage of #![doc(html_root_url)] #185

Closed jplatte closed 3 years ago

jplatte commented 3 years ago

@KodrAus @SirWindfield is one of you aware why we have #![doc(html_root_url = "https://docs.rs/env_logger/0.8.1")] at the crate root? I find it a bit annoying to have to bump that when releasing a new version.

mainrs commented 3 years ago

So that cross crate linking in documentation works.

jplatte commented 3 years ago

How does that work? Can you give an example of where the env_logger docs contain a cross-crate link, or some general docs about #[doc(html_root_url)] w.r.t. cross-crate links on docs.rs?

mainrs commented 3 years ago

Not sure, I never used it. But it's listed under the API guidelines for Rust: https://rust-lang.github.io/api-guidelines/documentation.html#crate-sets-html_root_url-attribute-c-html-root

I only found this issue: https://github.com/rust-lang/api-guidelines/issues/186

jplatte commented 3 years ago

Okay, so basically a workaround for a docs.rs issue? And even one that we are very unlikely to hit with a lib that's not really meant to be part of other crates' public APIs? @jyn514 can you confirm?

mainrs commented 3 years ago

To be honest, I don't see any reason to remove it. Who knows who might actually rely on this. Like, some internal documentation for projects might link back to this or so.

It's not a workaround, I think it's the way it's supposed to work. The issue is about having documentation on the style guidelines regarding it in general.

jplatte commented 3 years ago

The reason I'd like to get rid of it is that with it, a release takes longer (even if just a few seconds) and is easier to screw up.

It's not a workaround, I think it's the way it's supposed to work.

This makes no sense to me. There is no information that attribute adds that a tool like docs.rs couldn't theoretically infer.

jyn514 commented 3 years ago

I don't like that suggestion on the API guidelines. From the rustdoc documentation:

The #[doc(html_root_url = "…")] attribute value indicates the URL for generating links to external crates. When rustdoc needs to generate a link to an item in an external crate, it will first check if the extern crate has been documented locally on-disk, and if so link directly to it. Failing that, it will use the URL given by the --extern-html-root-url command-line flag if available. If that is not available, then it will use the html_root_url value in the extern crate if it is available. If that is not available, then the extern items will not be linked.

This information has nothing to do with building documentation on docs.rs. This is for when you build locally with doc --no-deps. I agree with @jplatte that it's a workaround, the proper solution would be https://github.com/rust-lang/cargo/issues/8296.

mainrs commented 3 years ago

My bad then.

jyn514 commented 3 years ago

This is for when you build locally with doc --no-deps

To clarify, the scenario where this goes wrong is:

  1. Another downstream crate uses env_logger as a dependency
  2. downstream re-exports or in some other way links to env_logger
  3. A user or developer of downstream builds documentation locally with cargo doc --no-deps (without --extern-html-root-url, because in practice no one but docs.rs does that).

Then the links to env_logger will be broken. But there's a simple fix and the fix is to remove --no-deps.

jplatte commented 3 years ago

Thanks for the clarification!

I don't like that suggestion on the API guidelines.

Do you think it would be worthwhile to try changing this part of the API guidelines?

Either way, I'll remove the attribute from env_logger.

jyn514 commented 3 years ago

I only found this issue: rust-lang/api-guidelines#186

That's not really relevant to html_root_url, it's about hand-written links, not the links rustdoc generates automatically.

Do you think it would be worthwhile to try changing this part of the API guidelines?

Sure, I'll look into it.

mainrs commented 3 years ago

That's not really relevant to html_root_url, it's about hand-written links, not the links rustdoc generates automatically.

I know, should have written it more explicit. I didn't find anything when googling.