Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move to doc links inside the prelude #75368

Merged
merged 2 commits into from
Aug 12, 2020

Conversation

poliorcetics
Copy link
Contributor

Helps with #75080.

@rustbot modify labels: T-doc, A-intra-doc-links, T-rustdoc

@rustbot rustbot added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 10, 2020
@rust-highfive
Copy link
Collaborator

r? @LukasKalbertodt

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 10, 2020

@bors r+ rollup

Good catch with the module@vec :)

@bors
Copy link
Contributor

bors commented Aug 10, 2020

📌 Commit 8ff768e21d93bc1cf78e89a4925a4f28ec314cfa has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2020
@poliorcetics
Copy link
Contributor Author

It's not my achievement, rustdoc caught the ambiguity by itself, which is amazing !

@poliorcetics
Copy link
Contributor Author

It fails on the module@crate::vec on CI but doesn't on my machine. @jyn514 any idea on how to fix this ?

@jyn514
Copy link
Member

jyn514 commented Aug 10, 2020

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 10, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 10, 2020

@poliorcetics try rebasing over master? I might have broken something in #75079 or #75318.

@poliorcetics
Copy link
Contributor Author

Rebasing did nothing (my branch is an hour old at most at this point) and rebuilding from scratch still worked when doing ./x.py doc library/std --stage 1.

@jyn514
Copy link
Member

jyn514 commented Aug 10, 2020

Not sure then, I'll have to debug it. I wish we had #75305 :(

@poliorcetics
Copy link
Contributor Author

poliorcetics commented Aug 10, 2020

Exact error:

error: `[crate::vec]` cannot be resolved, ignoring it.
  --> library/std/src/prelude/mod.rs:78:19
   |
78 | //! [`std::vec`]: module@crate::vec
   |                   ^^^^^^^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = note: `-D intra-doc-link-resolution-failure` implied by `-D warnings`
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

error: aborting due to previous error

I see a [crate::vec] at the top which makes little sense to me, it is expected ?

@poliorcetics poliorcetics force-pushed the intra-doc-links-std-prelude branch from 8ff768e to a6e492b Compare August 10, 2020 19:18
@LukasKalbertodt
Copy link
Member

Looks like this PR is already in good hands :)

r? @jyn514

@jyn514
Copy link
Member

jyn514 commented Aug 11, 2020

CI is documenting using stage 0, which doesn't have the latest fixes to intra-doc links. Not sure what this exact failure is from, but this will break anyway when we start linking to associated items (#74489).

@Mark-Simulacrum do you know why mingw-check uses doc --stage 0 library/std? Is it just for efficiency? I think we should either add -A broken_intra_doc_links or skip the check altogether if --stage 1 is too expensive to run.

@jyn514
Copy link
Member

jyn514 commented Aug 11, 2020

This would probably work if it went through a full bors build, but then the whole compiler will have broken mingw-check builds on every PR, so I don't want to do that unilaterally.

@jyn514
Copy link
Member

jyn514 commented Aug 11, 2020

This was added in 1f7c896. @ecstatic-morse, what was the rationale for using --stage 0 instead of --stage 1?

@poliorcetics
Copy link
Contributor Author

The PR that made the change was #71649 if that helps.

@Mark-Simulacrum
Copy link
Member

--stage 0 is intentionally checked (other stages are checked already on other builders) because we want to ensure that docs are buildable and viewable locally; this means that docs contributors don't need to build anything locally.

In the future I'd like for that to not be necessary, instead that we provide an easy way for a "recent master build" or perhaps nightly to work, but as of now that's not really feasible.

@poliorcetics
Copy link
Contributor Author

I moved the std::vec link back to a path, to make it compile and allow this to be merged.

@jyn514 do you think making a note in the tracking issue is possible (both for the affected files as they are found and a more generic one to warn people) ?

@jyn514
Copy link
Member

jyn514 commented Aug 11, 2020

we want to ensure that docs are buildable and viewable locally

Hmm ... if the intra-doc link breaks it will still be viewable, there will just be some broken links. What do you think about having cfg_attr(bootstrap, allow(broken_intra_doc_links))? That way x.py doc still works but we still ensure we have working links on nightly.

Merging this PR since it has good changes :) but I think it's worth continuing the discussion.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 11, 2020

📌 Commit 3ff06a9 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 11, 2020
@Mark-Simulacrum
Copy link
Member

I would prefer to avoid that - we only have a few weeks to go - but if it is truly necessary, then it should go into bootstrap/builder.rs along with other warnings. I can review such a PR.

@jyn514
Copy link
Member

jyn514 commented Aug 12, 2020

Well it's not certain that #74489 will get merged before the beta cutoff ... but other issues seems rare enough that we can delay the discussion until then, I agree if it's only 1 or 2 weeks it's not worth special casing and we can just do it after the new release.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2020
Rollup of 9 pull requests

Successful merges:

 - rust-lang#74521 (older toolchains not valid anymore)
 - rust-lang#74960 (Fix regionck failure when converting Index to IndexMut)
 - rust-lang#75234 (Update asm! documentation in unstable book)
 - rust-lang#75368 (Move to doc links inside the prelude)
 - rust-lang#75371 (Move to doc links inside std/time.rs)
 - rust-lang#75394 (Add a function to `TyCtxt` for computing an `Allocation` for a `static` item's initializer)
 - rust-lang#75395 (Switch to intra-doc links in library/std/src/os/*/fs.rs)
 - rust-lang#75422 (Accept more safety comments)
 - rust-lang#75424 (fix wrong word in documentation)

Failed merges:

r? @ghost
@bors bors merged commit c423fde into rust-lang:master Aug 12, 2020
@poliorcetics poliorcetics deleted the intra-doc-links-std-prelude branch August 12, 2020 06:49
@jyn514 jyn514 removed the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 25, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants