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.

Sign up for GitHub

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

Jump to bottom

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

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.

Sign in 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
Contributor

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
Contributor

bors commented Sep 3, 2020

⌛ Testing commit d715015 with merge 22967492ab8fd2d94ffafc1651b6b551ca2edaaa...

@bors
Copy link
Contributor

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
Contributor

bors commented Sep 3, 2020

⌛ Testing commit d715015 with merge c137084a9ed560b4e783ab4b0e8b2d2578821dbd...

@bors
Copy link
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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