-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 replace inet.af/netaddr with net/netip #7117
🌱 replace inet.af/netaddr with net/netip #7117
Conversation
011aca4
to
09a4227
Compare
@@ -162,6 +161,4 @@ require ( | |||
github.com/google/gnostic v0.5.7-v3refs // indirect | |||
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect | |||
github.com/pelletier/go-toml/v2 v2.0.1 // indirect | |||
go4.org/intern v0.0.0-20211027215823-ae77deb06f29 // indirect | |||
go4.org/unsafe/assume-no-moving-gc v0.0.0-20220617031537-928513b29760 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That one sounds like fun :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol
If your Go package wants to declare that it plays unsafe games that only work if the Go runtime's garbage collector is not a moving collector, then add:
import _ "go4.org/unsafe/assume-no-moving-gc"
Then your program will explode if that's no longer the case. (Users can override the explosion with a scary sounding environment variable.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no API.
From the godoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably good that we're not using it anymore
Thank you! /lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -32,7 +32,6 @@ require ( | |||
golang.org/x/net v0.0.0-20220812174116-3211cb980234 | |||
golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb | |||
google.golang.org/grpc v1.47.0 | |||
inet.af/netaddr v0.0.0-20220617031823-097006376321 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@schrej Doesn't matter at all. But are you also confused why we didn't have to bump go 1.17
in the go.mod file to use netip.ParseAddr
? :)
Or is go 1.18
more like enabling features of the language (like language level in Java (?) for e.g. generics) and you get the new Go SDk as soon as you go build
with Go 1.18?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. The compiler does heavily complain about generics if it doesn't say 1.18
in go.mod
, but it doesn't seem to care about packages that aren't available in an earlier release.
It doesn't compile with 1.17, so technically the project needs to change to 1.18
in go.mod
, or revert this change. Probably doesn't have any impact to consumers of APIs though, as its unlikely that someone imports the webhooks 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue to add linting for it: golang/go#46136
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for looking that up!
I think for CAPI main it's fine. We're probably days away from bumping to Go 1.19
If there are complaints in the meantime we can bump to 1.18
What this PR does / why we need it:
Replaces
inet.af/netaddr
withip/netip
, which was added with Go 1.18.