-
-
Notifications
You must be signed in to change notification settings - Fork 662
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
Update go vet with latest analyzers #3902
base: master
Are you sure you want to change the base?
Conversation
f592a57
to
4662b36
Compare
The nogo failure looks like it's easier to fix than ignore. |
@fmeum is it though? The Im in favor of disabling it with a TODO. |
True, I missed that this was the generated code, not the test. In that case I agree that disabling it by default is the right move. |
Yeah. The other option is to add a relevant I don't think it's worth the noise it gonna generate for downstream users though. |
989e850
to
690aa4b
Compare
Hmm, these dependencies upgrades turn out to be a huge PITA. Especially, for the I also had to patch Gazelle temporarily to avoid analyzer duplication. It should be a temp workaround until (a) we remove that in Gazelle, then (b) release a new Gazelle version, and then (c) upgrade the Gazelle version in rules_go. |
Upgrade golang.org/x/tools to v0.18.0. Note that the latest v0.19.0 has a bug that we should avoid using. Update the analyzers (passes) in TOOLS_NOGO and the ones automatically included when `vet = True` is configured. The vet analyzers are taken from `go tool vet help` documentation.
690aa4b
to
05860f6
Compare
Upgrade golang.org/x/tools to v0.18.0.
Note that the latest v0.19.0 has a bug that we should avoid using.
Update the analyzers (passes) in TOOLS_NOGO and the ones automatically
included when
vet = True
is configured.The vet analyzers are taken from
go tool vet help
documentation.