Skip to content

Combine enhancements #8257

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Apr 2, 2025

  • chore: modernize on the whole repo
  • chore: update golangci-lint to v2
  • upgrade golangci-lint-action
  • upgrade more golangci-lint-action
  • upgrade linter and add maintidx

@gjermundgaraba
Copy link
Contributor

me gusta

Would be nice to do this without upgrading to go 1.24 just yet though. Ideally I want to do that together with the SDK upgrading to 1.24

@faddat
Copy link
Contributor Author

faddat commented Apr 2, 2025

@gjermundgaraba that'd change the set of changes, how about:

  • make the changes 1.24 style
  • drop to 1.23

....work for you?

@gjermundgaraba
Copy link
Contributor

I am not sure I understand exactly what you mean by that. As long as the go version in go.mod and the github workflows stay at their current version I am good

@gjermundgaraba
Copy link
Contributor

Heyo! Some of these have already been merged (golangci-lint and modernize). Something I think we still want to do is look into some of the new linters enabled in golangci-lint 2 and see if there are any reasonable ones we are missing. Looking at #8195, I'd say at least copyloopvar is needed

@faddat
Copy link
Contributor Author

faddat commented Apr 3, 2025

oh, OK :)

I will do the thing and nice work dude!

BTW, what's modernize?

@faddat faddat force-pushed the faddat/combine-enhancements branch from 078f6b5 to 04f89a9 Compare April 3, 2025 05:05
@gjermundgaraba
Copy link
Contributor

modernize is essentially applying the nest gopls modernize suggestions

@gjermundgaraba
Copy link
Contributor

There has been quite a few changes that made some conflicts here. Not sure if you want to clean up the conflicts or open a new PR with just the linter additions? Would make the PR a bit cleaner potentially

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.

3 participants