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

Enable --generate-link-to-definition for rustc's docs #88292

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Enable --generate-link-to-definition for rustc's docs #88292

merged 1 commit into from
Sep 16, 2021

Conversation

SkiFire13
Copy link
Contributor

cc @jyn514

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 24, 2021
@jyn514
Copy link
Member

jyn514 commented Aug 24, 2021

r? @jyn514

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 24, 2021
jyn514
jyn514 reviewed Aug 24, 2021
View reviewed changes
src/bootstrap/builder.rs Outdated Show resolved Hide resolved
src/bootstrap/builder.rs Outdated Show resolved Hide resolved
@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 24, 2021
@SkiFire13 SkiFire13 force-pushed the enable-rustdoc-links branch 3 times, most recently from f40b46c to 3ef3cc7 Compare August 24, 2021 16:05
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member

jyn514 commented Aug 24, 2021 •
edited
Loading

src/test/helpers/exit_code.rs.html:32: broken link - std/os/imp/unix/process/trait.ExitStatusExt.html

Hmm, that looks like a bug in rustdoc - it shouldn't be generating links to private items. @GuillaumeGomez you may want to take a look

@jyn514 jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2021
@GuillaumeGomez
Copy link
Member

Funny one. The whole os module is cursed though. I'll open an issue to not forget.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 24, 2021
Closed
@GuillaumeGomez GuillaumeGomez mentioned this pull request Sep 3, 2021
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 11, 2021
@GuillaumeGomez
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
e0f6ff4 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc `@jyn514`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 11, 2021
@GuillaumeGomez
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
f064ad7 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc ``@jyn514``
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 12, 2021
@workingjubilee
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
9116186 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc `@jyn514`
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 14, 2021
@Manishearth
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
8be84a9 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc ``@jyn514``
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 14, 2021
@Manishearth
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
44cafcd 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc ```@jyn514```
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 14, 2021
@Manishearth
Rollup merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reex…
3cd22f9 
…ports, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc ````@jyn514````
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2021
@bors
Auto merge of rust-lang#88619 - GuillaumeGomez:simplify-std-os-reexpo…
e846f9c 
…rts, r=Amanieu

Remove `cfg(doc)` from std::os module reexports to fix rustdoc linking issues

Fixes rust-lang#88304.

I tested it based on rust-lang#88292.

Not sure if it's the best approach, but at least it makes thing a bit simpler.

cc `@jyn514`
@GuillaumeGomez GuillaumeGomez removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 15, 2021
@GuillaumeGomez
Copy link
Member

#88619 was merged so this PR is now unblocked. Please rebase and then we'll make a final review.

camelid
camelid reviewed Sep 15, 2021
View reviewed changes
src/bootstrap/builder.rs Outdated Show resolved Hide resolved
@camelid
Copy link
Member

camelid commented Sep 15, 2021

I would rather enable --generate-link-to-definition on just the rustc API docs to start with. There has not been an RFC or an FCP for --generate-link-to-definition, and enabling it for the std docs feels like we're committing to this feature somewhat.

@GuillaumeGomez
Copy link
Member

I agree with @camelid. There are still a lot of things to do on this feature (apart from being on the road to stabilize it) so let's take one step at a time.

@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 16, 2021
@camelid
Copy link
Member

camelid commented Sep 16, 2021 •
edited
Loading

@SkiFire13 Could you change this to only enable it for the rustc docs? If you grep for --show-type-layout in src/bootstrap, just add another argument for --generate-link-to-definition at whatever places show up in the grep output (and then remove the current change to src/bootstrap/builder.rs).

@jyn514
Copy link
Member

jyn514 commented Sep 16, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2021

📌 Commit 549d742 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 Sep 16, 2021
@SkiFire13 SkiFire13 changed the title Enable --generate-link-to-definition for rustc and stdlib's docs Enable --generate-link-to-definition for rustc's docs Sep 16, 2021
@Manishearth Manishearth mentioned this pull request Sep 16, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 16, 2021
@bors
Auto merge of rust-lang#89019 - Manishearth:rollup-5qp8a5s, r=Manishe…
e4828d5 
…arth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#88292 (Enable --generate-link-to-definition for rustc's docs)
 - rust-lang#88729 (Recover from `Foo(a: 1, b: 2)`)
 - rust-lang#88875 (cleanup(rustc_trait_selection): remove vestigial code from rustc_on_unimplemented)
 - rust-lang#88892 (Move object safety suggestions to the end of the error)
 - rust-lang#88928 (Document the closure arguments for `reduce`.)
 - rust-lang#88976 (Clean up and add doc comments for CStr)
 - rust-lang#88983 (Allow calling `get_body_with_borrowck_facts` without `-Z polonius`)
 - rust-lang#88985 (Update clobber_abi list to include k[1-7] regs)
 - rust-lang#88986 (Update the backtrace crate)
 - rust-lang#89009 (Fix typo in `break` docs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0ad800c into rust-lang:master Sep 16, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 16, 2021
@SkiFire13 SkiFire13 deleted the enable-rustdoc-links branch September 17, 2021 08:16
@GuillaumeGomez GuillaumeGomez mentioned this pull request Sep 24, 2021
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid left review comments

@jyn514 jyn514 jyn514 left review comments

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@jyn514 jyn514

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.57.0
Development

Successfully merging this pull request may close these issues.
9 participants
@SkiFire13 @rust-highfive @jyn514 @rust-log-analyzer @GuillaumeGomez @camelid @bors @Mark-Simulacrum @rustbot