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

OsStr eq_ignore_ascii_case takes arg by value #81710

Merged
merged 1 commit into from
Feb 5, 2021
Merged

Conversation

TyPR124
Copy link
Contributor

@TyPR124 TyPR124 commented Feb 3, 2021

Per a comment on #70516 this changes eq_ignore_ascii_case to take the generic parameter S: AsRef<OsStr> by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie my_os_str.eq_ignore_ascii_case::<str>("foo") becomes my_os_str.eq_ignore_ascii_case::<&str>("foo").

Besides that, I believe it is overall more flexible since it can now take an owned OsString for example.

If this change should be made in some other PR (like #80193) then please just close this.

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
@rust-highfive
Copy link
Collaborator

r? @KodrAus

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 3, 2021
@TyPR124
Copy link
Contributor Author

TyPR124 commented Feb 3, 2021

pinging @dtolnay

@m-ou-se
Copy link
Member

m-ou-se commented Feb 3, 2021

This is technically a breaking change to an unstable method.

That's perfectly fine. That's what unstable things are for. :)

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 3, 2021

📌 Commit 4d1efb7 has been approved by m-ou-se

@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 Feb 3, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 3, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 4, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 4, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2021
Rollup of 9 pull requests

Successful merges:

 - rust-lang#74304 (Stabilize the Wake trait)
 - rust-lang#79805 (Rename Iterator::fold_first to reduce and stabilize it)
 - rust-lang#81556 (introduce future-compatibility warning for forbidden lint groups)
 - rust-lang#81645 (Add lint for `panic!(123)` which is not accepted in Rust 2021.)
 - rust-lang#81710 (OsStr eq_ignore_ascii_case takes arg by value)
 - rust-lang#81711 (add #[inline] to all the public IpAddr functions)
 - rust-lang#81725 (Move test to be with the others)
 - rust-lang#81727 (Revert stabilizing integer::BITS.)
 - rust-lang#81745 (Stabilize poison API of Once, rename poisoned())

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 21e5827 into rust-lang:master Feb 5, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants