Skip to content
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

Linter needs updates after moving master to go 1.21.x #806

Closed
dpippenger opened this issue Mar 21, 2024 · 0 comments · Fixed by #807
Closed

Linter needs updates after moving master to go 1.21.x #806

dpippenger opened this issue Mar 21, 2024 · 0 comments · Fixed by #807

Comments

@dpippenger
Copy link
Contributor

In October master was moved to go 1.21.1 #792 but this broke the linter due to new core types added in go 1.21. So all subsequent PR are failing lint checks. To resolve this the linter needs to be updated to at least version 1.54.0, which is the first version to support go 1.21
https://github.com/golangci/golangci-lint/releases/tag/v1.54.0

Unfortunately this update comes with some major changes to some of the linter modules. In particular depguard was enabled but there was no depguard config in the Burrow repo. Per upstream discussions depguard v1 with no config essentially did nothing, but with v2 it's deny by default so it's failing on all but core golang modules. To restore the previous behavior disabling the depguard module is probably the simplest approach
OpenPeeDeeP/depguard#49

The next major module issue is revive. The updated version of revive in the more recent golangci-lint fixed what was considered a bug which didn't enable the unused-parameters check by default.
golangci/golangci-lint#3653
mgechev/revive#799

This fails in a number of places in the Burrow code due to unused parameters. I believe the remediation of the unused parameters should be handled separately from moving to go 1.21.x and the updated version of golangci-lint since the issues are not new and unrelated to the 1.21.x move itself which is already merged to master. So I'll propose setting a config for revive that matches the prior version's default list of checks that excludes unused-parameters, but I'll also create an issue that suggests re-enabling the module and fixing the cases where it fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant