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

update to go 1.23 #5492

Merged
merged 2 commits into from
Nov 7, 2024
Merged

update to go 1.23 #5492

merged 2 commits into from
Nov 7, 2024

Conversation

crazy-max
Copy link
Member

No description provided.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

left two comments (probably not critical)

Dockerfile Outdated
@@ -17,7 +17,7 @@ ARG AZURITE_VERSION=3.18.0
ARG GOTESTSUM_VERSION=v1.9.0
ARG DELVE_VERSION=v1.22.1
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to update this one? I recall we had to update it in moby at some point due to incompatibility with go versions; moby/moby#48497

current version looks to be 1.23.1; https://github.com/go-delve/delve/releases/tag/v1.23.1

Copy link
Member

Choose a reason for hiding this comment

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

I also went looking for other related things, and was confused for a bit for the gopls version, because there's to v0.26, but it looks like we're using the golang.org/x/tools tag instead of the gopls/vXXX tags;

ARG GOPLS_VERSION=v0.26.0

Wondering if we should change that (https://github.com/golang/tools/releases/tag/gopls%2Fv0.16.2)

Copy link
Member Author

@crazy-max crazy-max Nov 7, 2024

Choose a reason for hiding this comment

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

Do we need to update this one?

Ah yes we should, there are fixes for Go 1.23 in latest stable, let me do that.

but it looks like we're using the golang.org/x/tools tag instead of the gopls/vXXX tags

I don't think we want specifically the gopls tool https://github.com/golang/tools/tree/master/gopls but tool repo to handle extra analyzers. Maybe GOTOOLS_VERSION would be a better name. cc @tonistiigi

Copy link
Member

Choose a reason for hiding this comment

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

Maybe GOTOOLS_VERSION would be a better name

That works as well 😅 I was just a bit confused "what's this version?" 😂

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@thompson-shaun
Copy link
Collaborator

Need to verify that qemu is good from #5255 (comment)

@thompson-shaun thompson-shaun mentioned this pull request Nov 7, 2024
@crazy-max
Copy link
Member Author

crazy-max commented Nov 7, 2024

Need to verify that qemu is good from #5255 (comment)

Has been fixed in 1.23.3: golang/go#68976 (comment) (https://go-review.googlesource.com/c/go/+/617716)

@tonistiigi tonistiigi merged commit be6f193 into moby:master Nov 7, 2024
91 checks passed
@crazy-max crazy-max deleted the go-1.23 branch November 7, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants