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

build!: update to golang 1.23 #733

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Conversation

blkperl
Copy link
Contributor

@blkperl blkperl commented Sep 30, 2024

No description provided.

@blkperl blkperl added dependencies PRs and issues specific to updating dependencies go Pull requests that update Go code labels Sep 30, 2024
@@ -1,6 +1,8 @@
module github.com/argoproj/pkg

go 1.14
go 1.21
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to bump the minimum supported Go version here? I mentioned this in #707 (comment) that CI and local use is one thing, but this actually impacts consumers / is a breaking change

Copy link
Contributor Author

@blkperl blkperl Oct 1, 2024

Choose a reason for hiding this comment

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

I'm pretty sure go mod tidy did it for me. I can revert the change if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, even running make lint will update the go.mod file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI fails without the go.mod update. I reverted the change

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks like because the toolchain config you added below requires Go 1.21+

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I didn't add it. golang automatically added it.

Copy link
Contributor

@agilgur5 agilgur5 Oct 1, 2024

Choose a reason for hiding this comment

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

Oh. Huh that was my first time seeing it and we don't have it in Workflows

I guess your newer version of Go will auto-add it? 🤔
Ironically you can't specify the 1.14 toolchain since that's older than 1.21

I'm fine with upgrading it, but since it will require consumers to have newer Go I'd prefer to avoid it if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, my local golang is 1.23 from homebrew.

Signed-off-by: william.vanhevelingen <[email protected]>

This comment was marked as resolved.

Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM. Not going to block on the minimum Go changes above because this is an internal package and older versions are EoL anyway

@agilgur5 agilgur5 merged commit 08edcb7 into argoproj:master Oct 4, 2024
5 checks passed
@agilgur5 agilgur5 changed the title build: update to golang 1.23 build!: update to golang 1.23 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies PRs and issues specific to updating dependencies go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants