Skip to content

Bump to Go 1.24, golangci-lint v2.2#6584

Merged
negz merged 6 commits intocrossplane:mainfrom
negz:y-u-do-this-golangci
Jul 3, 2025
Merged

Bump to Go 1.24, golangci-lint v2.2#6584
negz merged 6 commits intocrossplane:mainfrom
negz:y-u-do-this-golangci

Conversation

@negz
Copy link
Copy Markdown
Member

@negz negz commented Jul 2, 2025

Description of your changes

I've been avoiding this due to the breaking golangci-lint change.

This is a huge diff, but most of it is from golangci-lint run --fmt. I did touch a few files by hand and unfortunately they're in the same commit as the generated code. It's hard to avoid this, because golangci-lint run --fmt formats everything as soon as it runs (e.g. to check that my updates worked). I'll add comments on the things I touched manually.

Edit: I skimmed the diff, and it's 99% whitespace being added around blocks. I think this is all happening due to the gofumpt formatter. We had that enabled already, but it seems like perhaps it wasn't actually working until now?

I have:

Need help with this checklist? See the cheat sheet.

@negz negz requested review from a team, phisco and turkenh as code owners July 2, 2025 23:22
@negz negz requested a review from bobh66 July 2, 2025 23:22
negz added 5 commits July 2, 2025 16:22
Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
Signed-off-by: Nic Cope <nicc@rk0n.org>
I like the idea personally, but way too much churn to enable it.

Signed-off-by: Nic Cope <nicc@rk0n.org>
Seems like a good idea, but we do want reason to come before args etc in
test files.

Signed-off-by: Nic Cope <nicc@rk0n.org>
@negz negz force-pushed the y-u-do-this-golangci branch from 1d00c9e to 3a7be80 Compare July 2, 2025 23:22
99% of the code changes in this PR are auto-generated.

Signed-off-by: Nic Cope <nicc@rk0n.org>
@negz negz force-pushed the y-u-do-this-golangci branch from 3a7be80 to 54e0ba1 Compare July 2, 2025 23:40
@@ -1,16 +1,12 @@
run:
timeout: 10m
version: "2"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This file is pretty much entirely formatting changes.

I did disable 1-2 new linters.


package utils
// Package certificate contains utilities for working with test certificates.
package certificate
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I renamed this one package and its one function to avoid util as a package name, which is an anti-pattern that now gets caught by a linter.

var decodedPayload []byte
if val, ok := payloadData["payload"]; ok {
decodedPayload, err = base64.StdEncoding.DecodeString(val.(string))
decodedPayload, err = base64.StdEncoding.DecodeString(val.(string)) //nolint:forcetypeassert // TODO(negz): Will this always be a string?
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this ignore comment.

Copy link
Copy Markdown
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

these updated linters are very particular in their preference of new lines 🤓 - i suspect folks will run into that in a lot of upcoming PRs, but I'm not particularly concerned since we'll have consistency.

Seems reasonable to me! Any contributor advice to add, e.g. about updating their environments or anything?

@negz
Copy link
Copy Markdown
Member Author

negz commented Jul 3, 2025

Any contributor advice to add, e.g. about updating their environments or anything?

I think it's pretty IDE dependent. There's some info about v2 support at https://golangci-lint.run/welcome/integrations/ - notably it looks like you need to update to a pre-release version of the VS Code plugin for it to work.

@negz negz merged commit 0ca7bac into crossplane:main Jul 3, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants