-
Notifications
You must be signed in to change notification settings - Fork 19
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
Upgrade to golangci-lint v1.58.0 #611
Conversation
This jumps up two major versions and a few minor versions, so there are a few changes (I did the upgrade one version at a time).
@@ -42,6 +42,7 @@ linters: | |||
- bodyclose | |||
- containedctx | |||
- contextcheck | |||
- copyloopvar |
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.
copyloopvar
and intrange
are enabled, but just give a warning that they don't do anything unless using at least Go 1.22 (we're still using Go 1.17). We definitely don't want to upgrade beyond the features we need, since that would needlessly require users to upgrade.
@@ -112,6 +113,7 @@ linters: | |||
|
|||
# Deprecated by golangci-lint: | |||
- deadcode | |||
- execinquery |
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.
execinquery
has become deprecated.
@@ -140,7 +142,7 @@ linters: | |||
- goconst | |||
- gocyclo | |||
- godox | |||
- goerr113 | |||
- err113 |
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.
goerr113
was renamed by golangci-lint.
@@ -6,7 +6,7 @@ import ( | |||
"sort" | |||
) | |||
|
|||
//nolint:deadcode,unused | |||
//nolint:unused |
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.
deadcode
was already disabled, so the nolint
directive didn't need to include it.
@@ -153,7 +153,7 @@ func (c *GeoJSONFeatureCollection) UnmarshalJSON(p []byte) error { | |||
Type string `json:"type"` | |||
Features []GeoJSONFeature `json:"features"` | |||
} | |||
if err := json.Unmarshal(p, &topLevel); err != nil { | |||
if err := json.Unmarshal(p, &topLevel); err != nil { //nolint:musttag |
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.
I believe there's a false positive bug in musttag
that's triggered by this code.
I will try to create a minimum reproduction of the problem and, if successful, will raise a bug report.
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.
Reported here: go-simpler/musttag#93
@@ -71,7 +71,7 @@ func (w XY) Cross(o XY) float64 { | |||
// Avoid fused multiply-add by explicitly converting intermediate products | |||
// to float64. This ensures that the cross product is *exactly* zero for | |||
// all linearly dependent inputs. | |||
return float64(w.X*o.Y) - float64(w.Y*o.X) //nolint:unconvert | |||
return float64(w.X*o.Y) - float64(w.Y*o.X) |
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.
unconvert
doesn't give a false positive here anymore 😁
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 for reviewing @albertteoh ! 😁 |
Description
This jumps up two major versions and a few minor versions, so there are a few changes (I did the upgrade one version at a time).
Check List
Have you:
Added unit tests? N/A
Add cmprefimpl tests? (if appropriate?) N/A
Updated release notes? (if appropriate?) Yes.
Related Issue