Skip to content

Conversation

LeSeulArtichaut
Copy link
Contributor

I didn't have time to test this, so I will let the CI do it for me.

r? @jyn514 cc #75080

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2020
@jyn514 jyn514 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 20, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 20, 2020

30 second review: the errors from CI are


error: `[std::sync::Weak::upgrade]` cannot be resolved, ignoring it.
    --> library/alloc/src/sync.rs:1751:22
     |
1751 |     /// [`upgrade`]: std::sync::Weak::upgrade
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^ cannot be resolved, ignoring
     |
     = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

error: `[std::option::Option::None]` cannot be resolved, ignoring it.
    --> library/alloc/src/sync.rs:1750:19
     |
1750 |     /// [`None`]: std::option::Option::None
     |                   ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot be resolved, ignoring

Two points:

  1. std doesn't know it's own name. Use crate instead. (see also extern crate self as core in libcore is an ICE #73473, intra-rustdoc-link: Cannot use core in core crate #73445).
  2. Option and None are in scope from the prelude. So you can use [None] and [Option] instead of the full path.

@LeSeulArtichaut LeSeulArtichaut force-pushed the alloc-intra-doc branch 2 times, most recently from 9b4ee57 to 648bc51 Compare August 20, 2020 21:44
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks great, a couple suggestions. There's one broken link I'm very confused how rustdoc didn't catch.

@jyn514 jyn514 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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2020
@LeSeulArtichaut LeSeulArtichaut force-pushed the alloc-intra-doc branch 2 times, most recently from fbe9585 to 5ead554 Compare August 21, 2020 11:09
@jyn514
Copy link
Member

jyn514 commented Aug 21, 2020

@bors r+ rollup

Thanks for the PR!

@bors
Copy link
Collaborator

bors commented Aug 21, 2020

📌 Commit 5ead554a572cea8067e5633fe61b7500eb31fcf7 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 21, 2020
@LeSeulArtichaut
Copy link
Contributor Author

@jyn514 The following link isn’t resolved properly;

[`LinkedList::into_iter()`]�

Could you r-? I can’t fix it immediately

@jyn514
Copy link
Member

jyn514 commented Aug 21, 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 21, 2020
@LeSeulArtichaut
Copy link
Contributor Author

@jyn514 Should be good now 🤞

@jyn514
Copy link
Member

jyn514 commented Aug 21, 2020

@bors r+ rollup

Thanks for the PR! Congratulations on taking on so many files in your first intra-doc PR 😆

@bors
Copy link
Collaborator

bors commented Aug 21, 2020

📌 Commit 97072c6 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 21, 2020
@LeSeulArtichaut
Copy link
Contributor Author

Congratulations on taking on so many files in your first intra-doc PR 😆

That was nothing compared to #73622 🙄

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

Successful merges:

 - rust-lang#75705 (Move to intra-doc links for /library/core/src/intrinsics.rs)
 - rust-lang#75711 (Split `astconv.rs` into its own submodule)
 - rust-lang#75718 (Don't count variants/fields/consts/associated types in doc-coverage doc examples)
 - rust-lang#75725 (Use intra-doc-links in `alloc`)
 - rust-lang#75745 (Remove duplication in `fold_item`)
 - rust-lang#75753 (Another motivation for CFG: return-oriented programming)
 - rust-lang#75769 (Minor, remove double nesting of a test module)
 - rust-lang#75771 (Extend normalization in const-eval-query-stack test)
 - rust-lang#75781 (More inline asm register name fixups for LLVM)
 - rust-lang#75782 (Convert core/src/str/pattern.rs to Intra-doc links)
 - rust-lang#75787 (Use intra-doc-links in `core::ops::*`)
 - rust-lang#75788 (MIR call terminator represents diverging calls too)

Failed merges:

 - rust-lang#75773 (Introduce expect snapshot testing library into rustc)

r? @ghost
@bors bors merged commit ec62980 into rust-lang:master Aug 22, 2020
@LeSeulArtichaut LeSeulArtichaut deleted the alloc-intra-doc branch August 22, 2020 08:21
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

5 participants