-
Notifications
You must be signed in to change notification settings - Fork 27
Move to golangci v2 #608
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
Move to golangci v2 #608
Conversation
chombium
left a comment
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.
@jorbaum Thanks for your PR
Please take a look at the small things to be fixed.
src/cmd/cf-auth-proxy/main.go
Outdated
| client "code.cloudfoundry.org/go-log-cache/v3" | ||
| "code.cloudfoundry.org/log-cache/internal/auth" | ||
| . "code.cloudfoundry.org/log-cache/internal/cfauthproxy" | ||
| cfProxy "code.cloudfoundry.org/log-cache/internal/cfauthproxy" |
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.
If the linter complains about the dot import, simply remove the dot at use the package name, cfauthproxy in this case, instead before each call.
FYI. The package alias should be all lowercase and without underscores. You can find more info on package naming conventions here
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.
You are correct. Just to explain myself: I did it this way to avoid naming collisions with gateway, nozzle and cache vars and to avoid changing this local var name.
I now abbreviated the local vars instead.
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.
The linked blog post was unepectedly a nice read. Most of its recommendations not only make sense for Go development ;) .
src/internal/testing/auth.go
Outdated
| "strings" | ||
| "time" | ||
|
|
||
| // . imports are common usage of ginkgo |
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.
Please use . "github.com/onsi/gomega" //nolint:staticcheck without the extra // . imports are common usage of ginkgo comment
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.
Removed it.
Just to explain why I did it like that previously: My idea was that having a reason documented when ignoring rules makes it easier for others to understand why this decision was taken.
Noteworthy changes in golangci-lint v2: * New config format (migrated with `migrate` command) * Default timeout is now 0 instead of 1 minute. Also fixes issues reported by golangci-lint v2: * Do not use dot imports for sources under `cmd`. * Have nicer logs * Remove redundant type `int` * Use `fmt.Fprintf(x, ...)` instead of `x.Write(fmt.Sprintf(...))`
chombium
left a comment
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.
you did to much now :)
chombium
left a comment
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.
LGTM!
Description
Move to golangci-lint v2.
Issues were:
cmd.intfmt.Fprintf(x, ...)instead ofx.Write(fmt.Sprintf(...))Noteworthy changes in golangci-lint v2:
migratecommand)Type of change
Testing performed?
Checklist:
mainbranch, or relevant version branchIf you have any questions, or want to get attention for a PR or issue please reach out on the #logging-and-metrics channel in the cloudfoundry slack