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.

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

Advertise Vec in LinkedList docs #38297

Merged
merged 1 commit into from
Dec 25, 2016

Conversation

matklad
Copy link
Member

@matklad matklad commented Dec 11, 2016

r? @steveklabnik

Hi! We already advise to use Vec instead of LinkedList in the top-level collections documentation. But I think it may be missed by someone who just directly finds LinkedList.

What do you feel about advertising Vec directly in LinkedList docs as well?

@matklad
Copy link
Member Author

matklad commented Dec 11, 2016

Question: how should I cross-link the docs? Relative paths won't work for LinkedList structure, because it is visible as both collections::LinkedList and collections::linked_list::LinkedList.

@steveklabnik
Copy link
Member

Question: how should I cross-link the docs?

This is one area where we can't 😦

@ollie27
Copy link
Member

ollie27 commented Dec 13, 2016

You've hit #32130. The options are to leave the links out for now or add more exceptions to linkchecker here.

//! in constant time.
//!
//! Almost always it is better to use [`Vec`](../vec/struct.Vec.html) or
//! [`VecDeque`](./vec_deque/struct.VecDeque.html) instead of `LinkedList`.
Copy link
Member

Choose a reason for hiding this comment

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

Since this is a module-level doc comment, I guess LinkedList should have an url.

/// The `LinkedList` allows pushing and popping elements at either end
/// in constant time.
///
/// Almost always it is better to use [`Vec`](../vec/struct.Vec.html) or
Copy link
Member

Choose a reason for hiding this comment

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

We don't use this linking syntax, we use:

text long text etc [link] blabla
blablabla

[link]: url

/// in constant time.
///
/// Almost always it is better to use [`Vec`](../vec/struct.Vec.html) or
/// [`VecDeque`](./vec_deque/struct.VecDeque.html) instead of `LinkedList`.
Copy link
Member

Choose a reason for hiding this comment

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

Same comment.

//! in constant time.
//!
//! Almost always it is better to use [`Vec`](../vec/struct.Vec.html) or
//! [`VecDeque`](./vec_deque/struct.VecDeque.html) instead of `LinkedList`.
Copy link
Member

Choose a reason for hiding this comment

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

Same comment.

//! The `LinkedList` allows pushing and popping elements at either end
//! in constant time.
//!
//! Almost always it is better to use [`Vec`](../vec/struct.Vec.html) or
Copy link
Member

Choose a reason for hiding this comment

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

Same comment.

@matklad matklad force-pushed the linked-lists-are-not-cool branch 2 times, most recently from 1065395 to 8ea3d32 Compare December 14, 2016 12:26
@matklad
Copy link
Member Author

matklad commented Dec 14, 2016

So let's use links in the module docs, because they always work, and let's avoid links on the struct directly, because it is reexported.

@GuillaumeGomez
Copy link
Member

std/collections/linked_list/index.html:57: broken link - std/collections/linked_list/struct.LinkedList
collections/linked_list/index.html:56: broken link - vec/struct.Vec.html
collections/linked_list/index.html:57: broken link - collections/linked_list/struct.LinkedList

@matklad matklad force-pushed the linked-lists-are-not-cool branch 2 times, most recently from d46374d to 1c868ee Compare December 16, 2016 07:06
@matklad
Copy link
Member Author

matklad commented Dec 16, 2016

Linking to Vec is also impossible, because there are collections and std::collections. Should be good now?

/// The `LinkedList` allows pushing and popping elements at either end
/// in constant time.
///
/// Almost always it is better to use `Vec` or `VecDeque` instead of
Copy link
Member

Choose a reason for hiding this comment

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

Why not linking to VecDeque here? You did it just above.

Copy link
Member Author

@matklad matklad Dec 17, 2016

Choose a reason for hiding this comment

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

LinkedList struct is visible as both https://doc.rust-lang.org/std/collections/struct.LinkedList.html and https://doc.rust-lang.org/std/collections/linked_list/struct.LinkedList.html, so I can't craft a relative link that will be valid in both cases.

but for the linked_list module I can do this.

@matklad
Copy link
Member Author

matklad commented Dec 20, 2016

@GuillaumeGomez anything else to clean up here?

@GuillaumeGomez
Copy link
Member

Nothing, thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Dec 21, 2016

📌 Commit 71de148 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Dec 22, 2016

⌛ Testing commit 71de148 with merge d365d32...

@bors
Copy link
Contributor

bors commented Dec 22, 2016

💔 Test failed - auto-mac-32-opt

@steveklabnik
Copy link
Member

@bors: retry

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Dec 24, 2016
…r=GuillaumeGomez

Advertise Vec in LinkedList docs

r? @steveklabnik

Hi! We already [advise](https://doc.rust-lang.org/std/collections/#use-a-linkedlist-when) to use `Vec` instead of `LinkedList` in the top-level collections documentation. But I think it may be missed by someone who just directly finds `LinkedList`.

What do you feel about advertising `Vec` directly in `LinkedList` docs as well?
bors added a commit that referenced this pull request Dec 24, 2016
Rollup of 14 pull requests

- Successful merges: #37956, #38013, #38297, #38480, #38497, #38502, #38505, #38513, #38521, #38549, #38554, #38557, #38568, #38572
- Failed merges:
@bors bors merged commit 71de148 into rust-lang:master Dec 25, 2016
@matklad matklad deleted the linked-lists-are-not-cool branch July 9, 2019 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants