Skip to content

Fix intra-doc links on pub re-exports #76082

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

Jump to bottom
Merged
merged 3 commits into from
Sep 5, 2020
Merged

Fix intra-doc links on pub re-exports #76082

merged 3 commits into from
Sep 5, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Aug 29, 2020

Partial fix for #76073 - This removes the incorrect error, but doesn't show the documentation anywhere.
r? @GuillaumeGomez

@jyn514
[WIP] Fix intra-doc links on pub re-exports
1b19354 
This removes the incorrect error, but doesn't show the documentation
anywhere.
@jyn514 jyn514 added C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Aug 29, 2020
@rust-highfive

This comment has been minimized.

# to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2020
@jyn514 jyn514 mentioned this pull request Aug 29, 2020
Closed
@GuillaumeGomez
Copy link
Member

The current code looks good though. Ping me once it's fixed. ;)

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

@GuillaumeGomez the tests are failing because this documentation is never displayed. It seems weird that it's silently ignored, I can change the test but are you sure this should be merged as is?

@GuillaumeGomez
Copy link
Member

If the documentation isn't shown, it's invalid so please this issue first. ;)

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

We discussed this on Discord: the documentation is never shown for pub re-exports. That's not introduced by this PR, that's an existing behavior. So it's fine for now to just fix the bugs in intra-doc links, but we should have a conversation about whether we want to show docs on re-exports.

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

I think I can replicate the same bug with /// style comments on a module instead of //!, if so I'll change the test to that. If not, I'll just test that broken_intra_doc_links doesn't fire on the existing test.

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

I think I can replicate the same bug with /// style comments on a module instead of //!, if so I'll change the test to that

It's not possible to write /// style comments on a top-level modules 😆 So I just test that no false-positive errors are emitted.

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020 

warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/thread_local/1.0.1/download`, got 502
error: failed to download from `https://crates.io/api/v1/crates/thread_local/1.0.1/download`

@jyn514 jyn514 changed the title [WIP] Fix intra-doc links on pub re-exports Fix intra-doc links on pub re-exports Aug 30, 2020
@jyn514
Improve tests
d715015 
Now this actually tests the links are generated correctly
@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

Updated!

@ollie27 ollie27 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 2, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 3, 2020

ping @GuillaumeGomez - is this waiting on anything?

@GuillaumeGomez
Copy link
Member

Nope, all good!

@bors: r=ollie27,GuillaumeGomez

@bors
Copy link
Collaborator

bors commented Sep 3, 2020

📌 Commit d715015 has been approved by ollie27,GuillaumeGomez

@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 Sep 3, 2020
@bors
Copy link
Collaborator

bors commented Sep 3, 2020

⌛ Testing commit d715015 with merge 22967492ab8fd2d94ffafc1651b6b551ca2edaaa...

@bors
Copy link
Collaborator

bors commented Sep 3, 2020

💔 Test failed - checks-actions

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

kennytm commented Sep 3, 2020

@bors retry

2020-09-03T08:41:55.3983684Z  Downloading crates ...
2020-09-03T08:41:55.3983911Z error: failed to download from `https://crates.io/api/v1/crates/cloudabi/0.0.3/download`
2020-09-03T08:41:55.3984063Z 
2020-09-03T08:41:55.3984213Z Caused by:
2020-09-03T08:41:55.3984404Z   [55] Failed sending data to the peer (Connection died, tried 5 times before giving up)
2020-09-03T08:41:55.3984763Z ', src/tools/tidy/src/deps.rs:202:20

@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 Sep 3, 2020
@bors
Copy link
Collaborator

bors commented Sep 3, 2020

⌛ Testing commit d715015 with merge c137084a9ed560b4e783ab4b0e8b2d2578821dbd...

@bors
Copy link
Collaborator

bors commented Sep 3, 2020

💥 Test timed out

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

kennytm commented Sep 3, 2020

@bors retry

5 hour timeout on macOS x86_64-apple.

@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 Sep 3, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#76082 - jyn514:top-level-links, r=ollie27,G…
5a4e265 
…uillaumeGomez

Fix intra-doc links on pub re-exports

Partial fix for rust-lang#76073 - This removes the incorrect error, but doesn't show the documentation anywhere.
r? @GuillaumeGomez
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Sep 4, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#76082 - jyn514:top-level-links, r=ollie27,G…
aa6f881 
…uillaumeGomez

Fix intra-doc links on pub re-exports

Partial fix for rust-lang#76073 - This removes the incorrect error, but doesn't show the documentation anywhere.
r? @GuillaumeGomez
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Sep 4, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 5, 2020
@Dylan-DPC
Rollup merge of rust-lang#76082 - jyn514:top-level-links, r=ollie27,G…
956216e 
…uillaumeGomez

Fix intra-doc links on pub re-exports

Partial fix for rust-lang#76073 - This removes the incorrect error, but doesn't show the documentation anywhere.
r? @GuillaumeGomez
This was referenced Sep 5, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2020
@bors
Auto merge of rust-lang#76376 - Dylan-DPC:rollup-8chsbw9, r=Dylan-DPC
7d289ae 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#75695 (Add a regression test for issue-72793)
 - rust-lang#75741 (Refactor byteorder to std in rustc_middle)
 - rust-lang#75954 (Unstable Book: add links to tracking issues for FFI features)
 - rust-lang#75994 (`impl Rc::new_cyclic`)
 - rust-lang#76060 (Link vec doc to & reference)
 - rust-lang#76078 (Remove disambiguators from intra doc link text)
 - rust-lang#76082 (Fix intra-doc links on pub re-exports)
 - rust-lang#76254 (Fold length constant in Rvalue::Repeat)
 - rust-lang#76258 (x.py check checks tests/examples/benches)
 - rust-lang#76263 (inliner: Check for codegen fn attributes compatibility)
 - rust-lang#76285 (Move jointness censoring to proc_macro)

Failed merges:

r? @ghost
@bors bors merged commit f1eb5f8 into rust-lang:master Sep 5, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 5, 2020
@jyn514 jyn514 deleted the top-level-links branch September 5, 2020 17:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@ollie27 ollie27 ollie27 left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.
7 participants
@jyn514 @rust-highfive @GuillaumeGomez @bors @kennytm @ollie27 @rustbot