-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Bump Go and golang linter #4389
Conversation
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
7dc3bc9
to
5316d3b
Compare
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
go 1.21.8 | ||
go 1.22.4 |
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.
We can keep this on go1.21, unless the module can no longer work with go1.21 (which I doubt).
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.
Kinda wanted to bump it in prep for beta release 🤔
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.
right but the go mod now specifies the minimum required version; trying to use any package from the repo will not be rejected unless you're on go1.22.4
We can use go1.22 for building the binaries, but not sure if we need to enforce that version for using the module
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 I'm familiar, I think we had a similar discussion in #4347 RE: go toolchain version
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.
Then you can leave this line as is and add a line with toolchain go1.22.4
.
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.
yah, I think @Jamstah and I discussed that but I cant remember what made us stick with the existing setup 🤷♂️ maybe he can chime in
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.
Either way I think this should be merged :)
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.
Go 1.21 will only be supported until ~August, so I feel like its probably worth bumping it.
Any project using this repo as a dependency should be keeping currency anyway. I don't think we break anyone until they update the dependency anyway.
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.
Approved as is, or if the toolchain
version is used instead. We should keep up with the Go versions.
Bump Go, golangci-lint and alpine versions.
NOTE:
vet
linter has been renamed togovet
run
section in the configskip-dirs
has been renamed toexclude-dirs