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

Add Ipv6Addr::is_ipv4_mapped #119081

Merged
merged 1 commit into from
Jan 21, 2024
Merged

Add Ipv6Addr::is_ipv4_mapped #119081

merged 1 commit into from
Jan 21, 2024

Conversation

jstasiak
Copy link
Contributor

This change consists of cherry-picking the content from the original PR[1], which got closed due to inactivity, and applying the following changes:

  • Resolving merge conflicts (obviously)
  • Linked to to_ipv4_mapped instead of to_ipv4 in the documentation (seems more appropriate)
  • Added the must_use and rustc_const_unstable attributes the original didn't have

I think it's a reasonably useful method to have.

[1] #86490

This change consists of cherry-picking the content from the original
PR[1], which got closed due to inactivity, and applying the following
changes:

* Resolving merge conflicts (obviously)
* Linked to to_ipv4_mapped instead of to_ipv4 in the documentation (seems
  more appropriate)
* Added the must_use and rustc_const_unstable attributes the original
  didn't have

I think it's a reasonably useful method.

[1] rust-lang#86490
@rustbot
Copy link
Collaborator

rustbot commented Dec 18, 2023

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 18, 2023
@jstasiak
Copy link
Contributor Author

jstasiak commented Dec 18, 2023

One thing that's not obvious to me: is it fine to put a new method like this under the umbrella of issues (#76205, #27709) that currently don't know or say anything about such method?

What's the right procedure to do it right?

@tgross35
Copy link
Contributor

tgross35 commented Jan 19, 2024

I would put this under its own feature gate and tracking issue specifically so it doesn't get grouped into #27709, which has a lot of unknowns still (see #66584 and #76098). This is a lot more straightforward.

The referenced PR predates the ACP process so would you mind creating an ACP and linking it here? This is just an issue template at the libs team repo https://github.com/rust-lang/libs-team, it should be accepted pretty easily.

@rustbot label -T-libs +T-libs-api +S-waiting-on-ACP
@rustbot author
r? libs-api

@rustbot rustbot added S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 19, 2024
@rustbot rustbot assigned dtolnay and unassigned thomcc Jan 19, 2024
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2024
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you! This looks good to me. I am comfortable accepting this unstable addition — this is straightforward enough that I don't think an ACP would be needed to justify it.

Using #27709 for the tracking issue is fine. I have added Ipv6::is_ipv4_mapped to the method list there. We can (and have in the past) pick out a subset of methods from that tracking issue to stabilize faster, so grouping it together with less promising methods doesn't concern me.

@dtolnay dtolnay removed the S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. label Jan 21, 2024
@dtolnay
Copy link
Member

dtolnay commented Jan 21, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jan 21, 2024

📌 Commit 605504c has been approved by dtolnay

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#116090 (Implement strict integer operations that panic on overflow)
 - rust-lang#118811 (Use `bool` instead of `PartiolOrd` as return value of the comparison closure in `{slice,Iteraotr}::is_sorted_by`)
 - rust-lang#119081 (Add Ipv6Addr::is_ipv4_mapped)
 - rust-lang#119461 (Use an interpreter in MIR jump threading)
 - rust-lang#119996 (Move OS String implementation into `sys`)
 - rust-lang#120015 (coverage: Format all coverage tests with `rustfmt`)
 - rust-lang#120027 (pattern_analysis: Remove `Ty: Copy` bound)
 - rust-lang#120084 (fix(rust-analyzer): use new pkgid spec to compare)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3cd378c into rust-lang:master Jan 21, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Rollup merge of rust-lang#119081 - jstasiak:is-ipv4-mapped, r=dtolnay

Add Ipv6Addr::is_ipv4_mapped

This change consists of cherry-picking the content from the original PR[1], which got closed due to inactivity, and applying the following changes:

* Resolving merge conflicts (obviously)
* Linked to to_ipv4_mapped instead of to_ipv4 in the documentation (seems more appropriate)
* Added the must_use and rustc_const_unstable attributes the original didn't have

I think it's a reasonably useful method to have.

[1] rust-lang#86490
@jstasiak
Copy link
Contributor Author

Thank you both, appreciate it.

@jstasiak jstasiak deleted the is-ipv4-mapped branch January 21, 2024 11:25
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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants