Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Simplify into_key_slice_mut #67725
Simplify into_key_slice_mut #67725
Changes from all commits
37b5cca
9b92bf8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
We? I'd prefer something more objective like the slice or whatever it is
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.
I didn't actually write that comment, and think it's somewhat superfluous, but please come up with a concrete suggestion here, like:
self
cannot be the shared rootNodeRef
cannot be the shared rootThere 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.
I would go with:
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.
I don't think that's any better, on the contrary, it comes across as poor grammar and capitalization to me. Also, there are already at least 3 comments of this "we = self" style in the file, so at least one author and one reviewer didn't object.
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.
The style of the public API documentation of Option, Vector, BTreeSet, leads to (in unscientifically determined order of popularity):
NodeRef
cannot be the shared rootIf there are multiple instances involved, we explicitly use
self
(with backticks). Somehow, its awkward capitalization is carefully avoided, except Vec::split_off