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

Renamed AsciiStr::to_lower and AsciiStr::to_upper #17947

Merged
merged 1 commit into from
Oct 16, 2014

Conversation

lukemetz
Copy link

AsciiStr::to_lower is now AsciiStr::to_lowercase and AsciiStr::to_upper is AsciiStr::to_uppercase to match Ascii trait.

Part of issue #17790.

This is my first pull request so let me know if anything is incorrect.

Thanks!

[breaking-changes]

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

@@ -319,12 +319,20 @@ pub trait AsciiStr {
/// Convert to a string.
fn as_str_ascii<'a>(&'a self) -> &'a str;

/// Convert to vector representing a lower cased ascii string.
#[allow(missing_doc)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the convention we've been adopting here is to make the doc comment the deprecated message.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Should I squash then push -f to force update so they are squashed on my remote?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that should work.

@mahkoh
Copy link
Contributor

mahkoh commented Oct 11, 2014

Why is it to_lowercase and not to_lower? I know that the Char trait has the longer names, but I doubt the ergonomics of such expressions.

@lukemetz
Copy link
Author

@mahkoh
This was to match the changes that were made in #14401 to be more consistent in the Ascii module and suggested by #17790. Feel free to discard if this is unwanted.

@sinistersnare
Copy link
Contributor

should this have a [breaking-changes] note?

@lukemetz
Copy link
Author

@sinistersnare Done

@lukemetz lukemetz force-pushed the master branch 2 times, most recently from 771d3c6 to 29cd4dc Compare October 13, 2014 01:00
@aturon
Copy link
Member

aturon commented Oct 15, 2014

@lukemets: the [breaking-change] (note -- no s) needs to go on the commit message, not the PR comment, so that you can search it with git log.

Otherwise, this looks good to me!

Let me know when you've updated the message, and I'll send it to @bors.

Now AsciiStr::to_lowercase and AsciiStr::to_uppercase to match Ascii trait.
[breaking-change]
@lukemetz
Copy link
Author

@aturon Done! Thanks for the help!

bors added a commit that referenced this pull request Oct 16, 2014
AsciiStr::to_lower is now AsciiStr::to_lowercase and AsciiStr::to_upper is AsciiStr::to_uppercase to match Ascii trait.

Part of issue #17790.

This is my first pull request so let me know if anything is incorrect.

Thanks!

[breaking-changes]
@bors bors closed this Oct 16, 2014
@bors bors merged commit 0ad6f0a into rust-lang:master Oct 16, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
Always show error lifetime arguments as `'_`

Fixes rust-lang#17947

Changed error lifetime argument presentation in non-test environment to `'_` and now showing them even if all of args are error lifetimes.

This also influenced some of the other tests like `extract_function.rs`, `predicate.rs` and `type_pos.rs`. Not sure whether I need to refrain from adding lifetimes args there. Happy to fix if needed
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.

8 participants