Skip to content
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

CLOUDP-269300: Bump to Go 1.23 #1841

Merged
merged 9 commits into from
Oct 1, 2024
Merged

CLOUDP-269300: Bump to Go 1.23 #1841

merged 9 commits into from
Oct 1, 2024

Conversation

roothorp
Copy link
Collaborator

Including the relevant golangci version bump to support Go 1.23.
Also includes other miscellaneous version updates for devbox (these will be automated similar to dependabot in future).

wrt exportloopref:

Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.

All Submissions:

  • Have you signed our CLA?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if there is one).
  • Update docs/release-notes/release-notes-template.md if your changes should be included in the release notes for the next release.

@josvazg
Copy link
Collaborator

josvazg commented Sep 30, 2024

@roothorp I would make this a draft until we figure out the licenses issue. BTW, I can take this PR if you want.

@josvazg
Copy link
Collaborator

josvazg commented Sep 30, 2024

Taking this from @roothorp will make it a draft while I figure out the licenses issue

@josvazg josvazg marked this pull request as draft September 30, 2024 09:32
Comment on lines +130 to +132
export GOOS=linux
export GOARCH=amd64
go run github.com/google/$(GO_LICENSES)@v$(GO_LICENSES_VERSION) csv --include_tests $(BASE_GO_PACKAGE)/... > licenses.csv
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not the prettiest fix, but the only one I could find.

From here:
google/go-licenses#149 (comment)

Seems we won't be able to use devbox/nix for the go-licenses binary and instead need to use it as a module dep.

cc @roothorp

@josvazg josvazg marked this pull request as ready for review September 30, 2024 17:52
@josvazg josvazg self-requested a review September 30, 2024 17:52
Copy link
Contributor

github-actions bot commented Sep 30, 2024

Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

one nit, else lgtm 👍

@josvazg
Copy link
Collaborator

josvazg commented Oct 1, 2024

one nit, else lgtm 👍

Fixes applied

@josvazg
Copy link
Collaborator

josvazg commented Oct 1, 2024

Actually @s-urbaniak , we need to roll that last fix back:

pkg/controller/atlasproject/custom_roles.go:203:28: printf: fmt.Errorf format %w has arg stat.Error of wrong type string (govet)
				err = errors.Join(err, fmt.Errorf("%w", stat.Error))

stat.Error is not an error but a stringso @roothorp 's initial implementation was totally correct.

Revert to use `%s` again, because `stat.Error` is a `string` and thus cannot be wrapped as an error.
@josvazg josvazg merged commit 2921cce into main Oct 1, 2024
57 checks passed
@josvazg josvazg deleted the CLOUDP-269300-go-1.23 branch October 1, 2024 16:15
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.

4 participants