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

Fix IpAddr link when IpAddr moves to core #3519

Closed
wants to merge 1 commit into from

Conversation

faern
Copy link

@faern faern commented Jan 25, 2023

Small change needed in order to make CI pass over at rust-lang/rust#104265

@faern faern changed the title Fix IpAddr link Fix IpAddr link when IpAddr moves to core Jan 25, 2023
@faern
Copy link
Author

faern commented Jan 26, 2023

The CI breaks this PR because I make the link broken. But the CI in the other PR also breaks because I make the link broken. It's a catch 22. We need to break it here first so that the other PR can be merged in order to fix it again?

@carols10cents
Copy link
Member

carols10cents commented Jan 26, 2023

I note that your PR says:

Implements the (not yet approved) RFC rust-lang/rfcs#2832.

I don't want to merge this until the RFC gets approved. I can look into what it would take to ignore that link, or less ideally ignore CI here, when that's the case.

But actually, isn't this change going to break any links that are out there to https://doc.rust-lang.org/stable/std/net/enum.IpAddr.html ? Shouldn't we figure out how to do a redirect to avoid breaking those links?

@faern
Copy link
Author

faern commented Jan 26, 2023

I note that your PR says:

Implements the (not yet approved) RFC rust-lang/rfcs#2832.

The FCP has completed with the disposition to merge. So I dunno, I think it's currently just waiting for this PR to be ready to merge. I wrote that PR description before the FCP completed.

@pitaj
Copy link

pitaj commented Jan 29, 2023

Is this PR still needed? Looks like rust-lang/rust#104265 no longer uses a type alias for this.

@faern
Copy link
Author

faern commented Jan 29, 2023

No longer needed if the other PR is accepted as is. I have not yet heard any feedback on this not-type-aliasing-solution so I have not closed this yet

@faern faern closed this Jan 31, 2023
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.

3 participants