Skip to content

Add primitive intra-links - #49459

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:primitive-intra-links
Mar 31, 2018
Merged

Add primitive intra-links#49459
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:primitive-intra-links

Conversation

@GuillaumeGomez

@GuillaumeGomez GuillaumeGomez commented Mar 28, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Part of #43466.

r? @QuietMisdreavus

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2018
QuietMisdreavus
QuietMisdreavus reviewed Mar 29, 2018
View reviewed changes

@QuietMisdreavus QuietMisdreavus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yay! The basic design looks good, but there are a couple things that i'd like to see differently.

Comment thread src/librustdoc/clean/mod.rs Outdated

@QuietMisdreavus QuietMisdreavus Mar 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't link to the stable std docs anywhere, since libstd itself has a doc_html_root set to the nightly docs. I would prefer if you could somehow grab the url root for std to combine with the "hand-made" primitive link, or at least always link to nightly.

Comment thread src/librustdoc/clean/mod.rs Outdated

@QuietMisdreavus QuietMisdreavus Mar 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If these listings are supposed to be related to each other, can you at least make it a &[(&str, Def)]? A map of some kind would be best, but since it's a const, the slice-of-tuples will work.

@GuillaumeGomez
GuillaumeGomez force-pushed the primitive-intra-links branch 2 times, most recently from 74426f7 to 039d291 Compare March 29, 2018 11:42
@GuillaumeGomez
GuillaumeGomez force-pushed the primitive-intra-links branch from 039d291 to 561e8ef Compare March 29, 2018 11:44
@GuillaumeGomez

GuillaumeGomez commented Mar 29, 2018

Copy link
Copy Markdown
Member Author

Updated.

@nrc nrc mentioned this pull request Mar 29, 2018
Closed
27 tasks
@QuietMisdreavus

QuietMisdreavus commented Mar 29, 2018

Copy link
Copy Markdown
Contributor

Looks great! Thanks for getting into this!

@bors r+

@bors

bors commented Mar 29, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 561e8ef has been approved by QuietMisdreavus

@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 Mar 29, 2018
@bors

bors commented Mar 31, 2018

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 561e8ef with merge 1c5283b...

bors added a commit that referenced this pull request Mar 31, 2018
@bors
Auto merge of #49459 - GuillaumeGomez:primitive-intra-links, r=QuietM…
1c5283b 
…isdreavus

Add primitive intra-links

Part of #43466.

r? @QuietMisdreavus
@bors

bors commented Mar 31, 2018

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 1c5283b to master...

@bors
bors merged commit 561e8ef into rust-lang:master Mar 31, 2018
@GuillaumeGomez
GuillaumeGomez deleted the primitive-intra-links branch March 31, 2018 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@QuietMisdreavus QuietMisdreavus QuietMisdreavus left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@QuietMisdreavus QuietMisdreavus

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GuillaumeGomez @QuietMisdreavus @bors @rust-highfive