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

Switch to josharian/native for native endian? #7

Closed
bradfitz opened this issue Dec 13, 2022 · 5 comments · Fixed by #8
Closed

Switch to josharian/native for native endian? #7

bradfitz opened this issue Dec 13, 2022 · 5 comments · Fixed by #8

Comments

@bradfitz
Copy link
Contributor

@hugelgupf, any chance this repo could switch to using https://pkg.go.dev/github.com/josharian/native

@josharian and I are trying to reduce the number of copies of this code in the Go ecosystem, per golang/go#57237

As of josharian/native#3 it has both a boolean const and a binary.ByteOrder var.

It also has a fallback to runtime-based detection if build tags get stale in the future.

If you're concerned about lacking control, I'm sure @josharian could grant you co-maintainership of the native repo.

Happy to send you a PR if you're cool with it. Or you can.

@bradfitz
Copy link
Contributor Author

Oh, I guess we can't outright remove it, as it has a bunch of callers, like github.com/insomniacslk/dhcp/dhcpv4 etc.

But you could use the constant in a func init which should get compiled away to ~nothing.

And you could update the docs to point people at the native package?

@josharian
Copy link

I'm sure @josharian could grant you co-maintainership of the native repo

Indeed I could, just say the word.

bradfitz added a commit to bradfitz/uio that referenced this issue Dec 13, 2022
bradfitz added a commit to bradfitz/uio that referenced this issue Dec 13, 2022
Fixes u-root#7
Updates golang/go#57237

Signed-off-by: Brad Fitzpatrick <brad@danga.com>
hugelgupf pushed a commit that referenced this issue Dec 13, 2022
Fixes #7
Updates golang/go#57237

Signed-off-by: Brad Fitzpatrick <brad@danga.com>
@hugelgupf
Copy link
Member

No need for co-maintainership, I'm happy with this. Thanks!

@hugelgupf
Copy link
Member

If you go make a PR to update dhcp, I'll be happy to approve that as well.

@bradfitz
Copy link
Contributor Author

If you go make a PR to update dhcp, I'll be happy to approve that as well.

Done: insomniacslk/dhcp#484

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 a pull request may close this issue.

3 participants