Skip to content
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

clarify default behavior of documentation field #13660

Closed
wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

@rustbot
Copy link
Collaborator

rustbot commented Mar 27, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2024
@epage
Copy link
Contributor

epage commented Mar 27, 2024

For myself, I feel like its a bug in the system that we have to add so much nuance here. If that bug isn't being fixed, should we just not reference this and encourage people to set the links?

@ehuss
Copy link
Contributor

ehuss commented Mar 27, 2024

Yea, I would agree this seems like weird bug to document. I'm also a little confused, as it seems like the accepted solution should be relatively trivial (something like documentation.or_else(|| format!("https://docs.rs/{name}/latest/{name}")) in the search code).

@RalfJung
Copy link
Member Author

See rust-lang/crates.io#1484, apparently what the crate page does is a lot more complicated than documentation.or_else and involves querying the docs.rs API to check if the crate is actually documented there.

@ehuss
Copy link
Contributor

ehuss commented Mar 27, 2024

My comment was based on rust-lang/crates.io#1484 (comment), which says that the search results should not do that query, and instead just always link to the latest version if the documentation field is not specified. That might cause an issue when the docs are not yet ready, or failed to build, but would be no different than if the user specified the documentation field manually.

@RalfJung
Copy link
Member Author

Yeah, agreed.

@weihanglo
Copy link
Member

If we agree on seeing a fix on crates.io or other services, should this be closed?

@weihanglo
Copy link
Member

Close as it is more like a buggy behavior in crates.io that we don't want to document. Thanks everyone for the discussion.

@RalfJung
Copy link
Member Author

Does the cargo team define whether this is a bug in crates.io? That seems like a crates.io decision to me. rust-lang/crates.io#1484 is marked as "enhancement".

Meanwhile, we are doing our users a disservice by having documentation that actively contradicts reality.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 18, 2024

Even if this is a bug in crates.io, it is clearly a long-standing issue and hard to fix. So telling users in the cargo docs that they can just leave the docs field empty if they use docs.rs (which is effectively what the docs are doing) is not helping.

I have to say I am quite disappointed with how this has been handled -- we have docs that actively confuse users and lead them in the wrong direction, and instead of trying to fix this we're squabbling about whether this is a bug or not?

It would be better to not mention this crates.io feature at all then: #14561.

bors added a commit that referenced this pull request Sep 18, 2024
remove reference to incomplete crates.io feature from docs

The cargo docs for the `documentation` field currently are written in a way that makes it sound like if I want to use docs.rs, I can just leave the field empty. However, that is not the case: leaving the field empty will never show a "Documentation" link in the search results, so there's always an unnecessary extra click to go from "type crate name into search" to reaching the docs.

This crates.io limitation is tracked at rust-lang/crates.io#1484. It doesn't really matter whether this is a bug or a missing feature, the point is that cargo docs are misrepresenting what crates.io does in a way that leads to a suboptimal user experience (many crates without "Documentation" link in crates.io search results). Since the suggestion to document what crates.io actually does was rejected (#13660), I suggest we instead stop mentioning this feature at all -- that's still clearly better than mentioning it while it is not yet fully implemented / while it has some significant *undocumented* limitation.
@epage
Copy link
Contributor

epage commented Sep 18, 2024

Meanwhile, we are doing our users a disservice by having documentation that actively contradicts reality.

Its fairly close to reality

Even if this is a bug in crates.io, it is clearly a long-standing issue and hard to fix. So telling users in the cargo docs that they can just leave the docs field empty if they use docs.rs (which is effectively what the docs are doing) is not helping.

While its long standing, there has been recent progress to fixing it. iiuc the blocker for fixing it is on docs.rs's side and the PR for that was started this summer (rust-lang/docs.rs#2533).

I mentioned that I was fine with removal if this was going to the status quo but that doesn't appear to be the case, so I lean towards keeping the documentation.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 18, 2024

It's far enough from reality to cause actual problems: many crates have no "Documentation" link in the crates.io search results, adding an entirely unnecessary extra click for the quite common flow of "go search some crate to find its docs". I think these docs (and the others that should still be fixed) take a good chunk of responsibility for that.

@epage
Copy link
Contributor

epage commented Sep 18, 2024

If the concern is about browsing crates.io, at this point, it would be a major undertaking to get people to notice the documentation change to specify package.documentation and for people to act on it. It doesn't seem worthwhile to make that shift when we'll just switch back again.

epage added a commit to epage/cargo that referenced this pull request Sep 18, 2024
This reverts commit 1735917.

As discussed in rust-lang#13660, my stance on removing the docs was if there
wasn't a path forward.
There is work progressing on the docs.rs side which will unblock the
crates.io side.

If the concern for removing the docs is for new crates, then no harm in
removing it for now but also little benefit.
If its to get existing crates to change, I don't think that ecosystem
churn is worth it to try to get everyone to add the link and then remove
it again.
bors added a commit that referenced this pull request Sep 18, 2024
Revert "remove reference to incomplete crates.io feature from docs"

This reverts commit 1735917.

As discussed in #13660, my stance on removing the docs was if there wasn't a path forward.
There is work progressing on the docs.rs side which will unblock the crates.io side.

If the concern for removing the docs is for new crates, then no harm in removing it for now but also little benefit.
If its to get existing crates to change, I don't think that ecosystem churn is worth it to try to get everyone to add the link and then remove it again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants