-
Notifications
You must be signed in to change notification settings - Fork 3.4k
restore full go vet behaviour #28945
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
restore full go vet behaviour #28945
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
9de604a
to
cfe4784
Compare
cfe4784
to
9d72400
Compare
Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Avoid copying the flow protobuf message, as this triggers go vet to complain about copying a mutex (in internal protobuf state). No functional changes intended, simply appeasing vet. Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Without -vet=all, go test doesn't run the same set of vets as go vet. Since we dropped go vet ./... from the integration tests, we weren't running the full suite anymore. Fixes: f6346d2 (make: drop redundant `go vet ./...` from integration tests) Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
9d72400
to
ca14e6d
Compare
/test |
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.
Hubble changes LGTM, thanks @bimmlerd!
@bimmlerd LGTM |
Tests reliant on 1.1.1.1 are failing due to https://www.cloudflarestatus.com/incidents/hm7491k53ppg, I'll wait with retriggering until that is resolved. |
/ci-ipsec-upgrade |
/ci-awscni |
/ci-clustermesh |
/ci-e2e |
/ci-eks |
/ci-ipsec-e2e |
This PR contains three commits to fix up
go vet ./...
which had regressed, and a fourth commit to reinstate running it as part of CI.This effectively reverts #26978, which seems fine as the PR which is linked as requiring it was never merged. Additionally, the work which seems to supersede it doesn't appear to require the deepcopy stuff #28873 (feel free to correct me here, @marqc).
cc @tklauser for the original change to drop
go vet
.