-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add an arch
package
#12
Conversation
882e268
to
d17b12c
Compare
Many users of this package will want to translate between Go and RPM architectures. This moves code that exists in coreos-assembler (and can then be deleted from there): https://github.com/coreos/coreos-assembler/blob/master/mantle/system/arch.go
d17b12c
to
2896621
Compare
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!
return m.rpmArch | ||
} | ||
} | ||
return goarch |
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.
Maybe this isn't a concern, but would the "pass through" assumption be potentially deceiving? e.g. one would assume whatever comes out of the RpmArch
function is definitely an RPM Arch.
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.
It's definitely a concern! The downside is that we'd need to maintain an exhaustive list here of all architectures to validate it, I didn't think that was worth the maintenance overhead.
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.
Sounds fair :)
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.
A good example is I notice the coreos-assembler code is already missing RISC-V and...honestly I'm not sure if that matches or not. OK looks like the Go arch is riscv64
and the RPM one is (time passes while I dig in links from here) that too! Hooray.
Many users of this package will want to translate between
Go and RPM architectures. This moves code that exists
in coreos-assembler (and can then be deleted from there):
https://github.com/coreos/coreos-assembler/blob/master/mantle/system/arch.go