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

util: copy go1.16 ParseIP into separate file #69664

Merged

Conversation

RichardJCai
Copy link
Contributor

@RichardJCai RichardJCai commented Aug 31, 2021

go1.17 is changing how IPs are parsed, leading 0s will not be accepted.
This change ensures that IP addresses with leading 0s are accepted
(which is the case in Postgres)

Release justification: only moved code out of go package to it's own
file.
Release note: None

@RichardJCai RichardJCai requested review from knz, rafiss and a team August 31, 2021 19:36
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RichardJCai RichardJCai force-pushed the copy_go1.16_ip_addr_parsing_08312021 branch from 479963c to 22779e8 Compare August 31, 2021 19:36
// ("2001:db8::68"), or IPv4-mapped IPv6 ("::ffff:192.0.2.1") form.
// If s is not a valid textual representation of an IP address,
// ParseIP returns nil.
func ParseIP(s string) IP {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we make ParseIP / IPv4 private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Realized ParseIP has to be used in tree as well so that has to be exported.

Changed IPv4 to makeIPv4.

@@ -0,0 +1,235 @@
// Copyright 2009 The Go Authors. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

probably need to silence the linter on the copyright thing
make a comment that this is a copy from the v1.16 code & why this is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@rafiss
Copy link
Collaborator

rafiss commented Sep 1, 2021

Take a look at https://github.com/cockroachdb/cockroach/blob/master/pkg/util/uuid/uuid.go to see an example of how to preserve the original copyright

@RichardJCai RichardJCai force-pushed the copy_go1.16_ip_addr_parsing_08312021 branch from 22779e8 to 1ba0487 Compare September 1, 2021 15:44
@blathers-crl blathers-crl bot requested a review from otan September 1, 2021 15:44
go1.17 is changing how IPs are parsed, leading 0s will not be accepted.
This change ensures that IP addresses with leading 0s are accepted
(which is the case in Postgres)

Release justification: only moved code out of go package to it's own
file.

Release note: None
@RichardJCai RichardJCai force-pushed the copy_go1.16_ip_addr_parsing_08312021 branch from 1ba0487 to f44ccb0 Compare September 1, 2021 22:50
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @otan and @rafiss)

@RichardJCai
Copy link
Contributor Author

Thanks for the reviews!
bors r+

@craig
Copy link
Contributor

craig bot commented Sep 7, 2021

Build succeeded:

@craig craig bot merged commit 1346d53 into cockroachdb:master Sep 7, 2021
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.

5 participants