-
Notifications
You must be signed in to change notification settings - Fork 13k
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
collections: update docs of slice get() and friends #39150
Conversation
for the new SliceIndex trait. Also made the docs of the unchecked versions a bit clearer; they return a reference, not an "unsafe pointer".
Just reopen the PR and talk about it with the one who closed it. Generally we ask a first time before closing, but it depends on the member. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few urls missing.
/// index. | ||
/// | ||
/// - If given a position, returns a reference to the element at that | ||
/// position or `None` if out of bounds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add None
url please?
/// - If given a position, returns a reference to the element at that | ||
/// position or `None` if out of bounds. | ||
/// - If given a range, returns the subslice corresponding to that range, | ||
/// or `None` if out of bounds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add None
url please?
@@ -360,7 +367,10 @@ impl<T> [T] { | |||
core_slice::SliceExt::get(self, index) | |||
} | |||
|
|||
/// Returns a mutable reference to the element at the given index. | |||
/// Returns a mutable reference to an element or subslice depending on the | |||
/// type of index (see [`get()`]) or `None` if the index is out of bounds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add None
url please?
Did you look at the closed PR? There is a reason why I did not put in the URLs. |
That's what I did, no? |
No I mean, the PR that been closed. You created a new one. However, maybe it wasn't possible to just reopen it? Not sure... |
Oh damn you're right. And no, I didn't look at the original PR before you said it. Then it's all good. Sorry for the incovenience. @bors: r+ rollup |
📌 Commit 871357a has been approved by |
Thanks! Sorry if I sounded a bit frustrated ;) |
It's understandable. Just try to speak with people if you have the feeling things aren't moving. There are a lot of PRs and work done but not always publicly. |
collections: update docs of slice get() and friends Resubmit of rust-lang#38216. r? @GuillaumeGomez BTW, instead of closing a PR just because it is old and the team member who offered to fix it up did not have the time to do so, why not ping them instead? (cc @alexcrichton)
Resubmit of #38216.
r? @GuillaumeGomez
BTW, instead of closing a PR just because it is old and the team member who offered to fix it up did not have the time to do so, why not ping them instead? (cc @alexcrichton)