-
Notifications
You must be signed in to change notification settings - Fork 38
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
pkg/cdi: fix build/vet failures on windows. #83
Conversation
d3fe743
to
b58232d
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
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.
Some minor comments. I think these are generally no-ops and I'd be happy to merge pending feedback.
(would be nice to update the dates in the license strings).
Fix the most obvious build failure on windows. Signed-off-by: Krisztian Litkey <[email protected]>
Signed-off-by: Krisztian Litkey <[email protected]>
Signed-off-by: Krisztian Litkey <[email protected]>
Signed-off-by: Krisztian Litkey <[email protected]>
Protect setting/reading external validator against read/write race. Signed-off-by: Krisztian Litkey <[email protected]>
5ec6131
to
584db3c
Compare
My preference would be to file another PR which removes any explicitly spelled out dates from all file headers and the copyright template (and create it if we don't have yet such a thing separately in the repo). This is similar to how, for instance, containerd deals with this. It removes this class of problem once and for all. |
That sounds great. |
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 @klihub.
Sorry about the back and forth on the minor points.
Fix two build failures for Windows. One in stock code and another in tests, which show up as a 'go vet' failure. Fix a data read/write race which was probably triggered by the latter Windows fix slightly altering timing in some of tests... or just by sheer luck.
Update the github workflow to run sanity checking (lint, vet) both for Linux and Windows. The checks always run on a Linux distro, but they override GOOS/GOARCH for the toolchain.
Update the github worfklow to do binary test builds/cross-builds both for Linux and Windows.