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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fromStatT() implementation for NetBSD #41441
Conversation
Signed-off-by: Leonardo Taccari <leot@NetBSD.org>
fromStatT()
implementation for NetBSD
fromStatT()
implementation for NetBSDThere 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.
looks like there's some duplication in the various fromStatT
implementations (e.g. freebsd and netbsd now look the same).
Perhaps someone wants to have a look at combining some of those in a follow-up.
LGTM
// fromStatT converts a syscall.Stat_t type to a system.Stat_t type | ||
func fromStatT(s *syscall.Stat_t) (*StatT, error) { | ||
return &StatT{size: s.Size, | ||
mode: uint32(s.Mode), |
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.
Looks like this conversion is redundant on netbsd, or did you add on purpose? (if so, perhaps this requires a //nolint:unconvert
)
mode: uint32(s.Mode), | ||
uid: s.Uid, | ||
gid: s.Gid, | ||
rdev: uint64(s.Rdev), |
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.
Same question here
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.
Replying to both questions here: yeah, both conversions should be useless and can be removed (I've also locally patched them and double-checked them).
@thaJeztah OK to open a new PR to remove them?
Thank you very much @viandc, @AkihiroSuda and @thaJeztah for the quick reviews and merge!
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.
If you have time to look at cleaning up the duplicates (I think if we keep the conversion in that one to take different cases into account, that's ok)
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.
Thanks @thaJeztah, I've just opened #41448 that does that.
- What I did
When packaging amazon-ecs-cli in pkgsrc-wip I have noticed build failure due non-existent
stat_netbsd.go
.- How I did it
On a NetBSD machine that supports Go, assuming pkgsrc tree and
pkgsrc-wip tree are checked out, try:
- How to verify it
Please see above.
- Description for the changelog
Add
fromStatT()
implementation for NetBSD- A picture of a cute animal (not mandatory but encouraged)
N/A, sorry! :)
But here a nice goat emoji!:
馃悙