-
Notifications
You must be signed in to change notification settings - Fork 4
feat(sggolangcilintv2): add golangci-lint v2 #661
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
base: master
Are you sure you want to change the base?
Conversation
ecfc792
to
96e6f65
Compare
1f9c6b8
to
1a48393
Compare
1d887db
to
879aba3
Compare
tools/sggolangcilint/tools.go
Outdated
cmd.Dir = directory | ||
return cmd | ||
} | ||
|
||
// Run GolangCI-Lint in every Go module from the root of the current git repo. | ||
func Run(ctx context.Context, args ...string) error { |
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.
This will be a breaking change for all clients, would be nice if clients that are using the default config today, doesnt need to do anything, and for clients that are using custom config, can supply a config.
Could maybe add RunWithConfig
?
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 guess that would mean adding a new function for each existing function, right?
That would be RunWithConfig
, FixWithConfig
, CommandWithConfig
, CommandInDirectoryWithConfig
, PrepareCommandWithConfig
.
And then I guess you would want the old Run
, Fix
etc hard-code a config that would be used, or did you want to keep using the old v1 config file?
Technically, the bump to v2 is a breaking change also for the CLI arguments which clients may be passing. We could add a whole new sggolangcilintv2
package instead, and you can keep using this sggolangcilint
(v1) as-is for as long as you want?
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've now created a sggolangcilintv2
package instead. Let me know what you think.
We could also deprecate the functions in the sggolangcilint
package, so to signal that one can move onto the v2 package.
I can also squash together the commits later. I just kept them now since we are iterating 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.
I think that is better, then clients can choose when to update 👍 So marking it as deprecated is good, so we eventually can remove it.
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.
Ok, now I have also deprecated sggolangcilint
. Let me know if you are missing anything.
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.
Tested it in pod-gateway, then it fails with deprecation warning 🤦♂️ maybe best to skip that initially then?
[sggolangcilint:prepare-command] fetching https://github.com/golangci/golangci-lint/releases/download/v1.64.8/golangci-lint-1.64.8-linux-amd64.tar.gz ...
[go-lint] main.go:132:9: SA1019: sggolangcilint.Run is deprecated: Use `sggolangcilintv2.Run` instead. Run GolangCI-Lint in every Go module from the root of the current git repo. (staticcheck)
[go-lint] return sggolangcilint.Run(
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.
Yeah, that was expected 😄 but sure, I removed that last commit now.
3ab2858
to
a55566f
Compare
a55566f
to
f6da29f
Compare
f6da29f
to
6b6d0f2
Compare
Why?
Golangci-lint was bumped to v2.
What?
sggolangcilintv2
as there are backwards braking changes to the CLI. The CLI arguments will differ from golangci-lint v1 and options around exclusion paths have been moved from the CLI so they must be defined in the config yaml.sggolangcilint
.golangci-lint migrate -c golangci.yml
. This sorted the linters for us, so they are now in alphabetical order. Thegci
,gofumpt
linters were removed (now exists as formatters) andgosimple
has been absorbed bystaticcheck
.Will be saved for a separate PR
Enabled vs disabled linters
Notes