-
Notifications
You must be signed in to change notification settings - Fork 101
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
proposal: support for net/netip types #88
Comments
I'd also like to see this. That said, I am planning on waiting until 1.19 is released and 1.17 has been end-of-lifed. |
Now that 1.19 is out and 1.17 is EOL, can we reconsider this? 🙏 |
Is there a particular use case beyond avoiding an |
Not really. I just had a cursory look at the code and naively expected the migration to be useful in a few cases. But nothing concrete.
Totally understandable 👍 |
@oschwald, sorry for the long delay.
Not just And it's not actually as easy, see golang/go#53607 and the related issues. In short,
It's hard to judge the real impact here without benchmarks, but since |
Also, As far as |
Maybe we can use |
I have started a v2 branch that includes this. All the tests are passing. As mentioned in the commit, there is no benefit in terms of performance when using When using the methods that return a network, e.g., Given the major version bump, I intend to introduce some other breaking changes before doing an actual release. I created a roadmap for this. |
Closing as this is done in the v2 beta. |
Go 1.18 includes the new
net/netip
package with value types related to IP addresses and networks.Perhaps, new methods should be added, for example
Reader.LookupAddr(ip netip.Addr, result any)
andReader.Prefixes
with a corresponding iterator, that would encourage the use of the new IP types. I think, it could also reduce some of the boilerplate such as:maxminddb-golang/reader.go
Lines 247 to 250 in ccd731c
and a lot of
len(ip) == net.IPv4len
andip.To4() == nil
.The text was updated successfully, but these errors were encountered: