Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Upgrade to latest Go, golangci-lint #408

Merged
merged 17 commits into from
Jun 21, 2024
Merged

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Jun 12, 2024

Go 1.19 is not supported anymore.
It won't receive security updates or bugfixes.

Upgrade to the latest release of Go 1.22,
and the latest golangci-lint to actually run with the new Go.

This required a number of linter fixups, or configuration adjustments.
These are detailed in individual commits in the PR.
I tried to make reasonable choices on those issues case-by-case.

I did not bump the 'go' directive in go.mod for now.

@abhinav abhinav marked this pull request as draft June 12, 2024 22:13
@abhinav
Copy link
Contributor Author

abhinav commented Jun 12, 2024

This also requires upgrading golangci-lint. Just a moment.

Go 1.19 is not supported anymore.
Upgrade to the latest release of Go 1.22.
They're now issues.exclude-files and exclude-dirs.
The linter is overly sensitive to numbers in source code
including file permissioning bits like 0700.
depguard v2 expects configuration to be supplied differently.
These linters have been deprecated and deactivated.
They can be removed from the list now.

```
WARN [lintersdb] The linter "maligned" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. https://golangci-lint.run/product/roadmap/#linter-deprecation-cycle
WARN [lintersdb] The linter "interfacer" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. https://golangci-lint.run/product/roadmap/#linter-deprecation-cycle
WARN [lintersdb] The linter "scopelint" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. https://golangci-lint.run/product/roadmap/#linter-deprecation-cycle
WARN [lintersdb] The linter "exhaustivestruct" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. https://golangci-lint.run/product/roadmap/#linter-deprecation-cycle
WARN [lintersdb] The linter "ifshort" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. https://golangci-lint.run/product/roadmap/#linter-deprecation-cycle
WARN [lintersdb] The linter "nosnakecase" is deprecated (step 2) and deactivated. It should be removed from the list of disabled linters. [..]
```
```
WARN [config_reader] The configuration option `linters.govet.check-shadowing` is deprecated. Please enable `shadow` instead, if you are not using `enable-all`.
```

Now the 'shadow' linter serves this purpose.
With enable-all, it's already enabled.
Unused parameters act as valid documentation.
Replacing them with "_" is undesirable.
```
manifest/manifesttest/pkgbuilder.go:53:1: Duplicate words (the) found (dupword)
util/filepatcher.go:39:2: assigned to result, but reassigned without using the value (wastedassign)
state/state.go:319:2: assigned to actualDigest, but reassigned without using the value (wastedassign)
```
```
ui/ui.go:189:10: fmt.Sprintf can be replaced with string concatenation (perfsprint)
ui/ui.go:197:10: fmt.Sprintf can be replaced with string concatenation (perfsprint)
envars/ops.go:303:42: fmt.Sprintf can be replaced with string concatenation (perfsprint)
manifest/actions.go:34:58: fmt.Sprintf can be replaced with string concatenation (perfsprint)
manifest/actions.go:63:10: fmt.Sprintf can be replaced with string concatenation (perfsprint)
manifest/actions.go:65:9: fmt.Sprintf can be replaced with string concatenation (perfsprint)
```
We could add struct tags to it, but I'm gonna say that's out of scope
for this PR.
It's deprecated and replaced with 'err113', and Hermit doesn't appear to
be failing that.
@abhinav abhinav changed the title Upgrade to Go 1.22 Upgrade to latest Go, golangci-lint Jun 12, 2024
@abhinav abhinav marked this pull request as ready for review June 12, 2024 23:05
@abhinav
Copy link
Contributor Author

abhinav commented Jun 12, 2024

This is ready for eyes now

Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay, LGTM!

@alecthomas alecthomas merged commit 1c0ace5 into cashapp:master Jun 21, 2024
6 checks passed
@abhinav abhinav deleted the go1.22 branch June 21, 2024 15:25
@abhinav
Copy link
Contributor Author

abhinav commented Jun 21, 2024

Apologies for the delay, LGTM!

Not a problem! Thanks for merging.

spicykay pushed a commit to spicykay/hermit that referenced this pull request Aug 23, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants