-
Notifications
You must be signed in to change notification settings - Fork 9
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
config.yaml: remove EOL go_versions #180
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the commit message a little more descriptive (i.e. let's mention golang versions).
go/tests.yaml
Outdated
@@ -60,6 +60,7 @@ files: | |||
go_test_cmd: ./test | |||
go_generated_dirs: [config, docs] | |||
go_generate_with_schematyper: true | |||
go_versions: [1.19.x, 1.20.x, 1.21.x] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instead we should probably be editing the defaults in config.yaml
so it applies to all golang projects in this repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, looking more closely, we're build with golang v1.20 in Fedora, CentOS Stream, and RHEL (at least for the streams we care about). So I think we can drop v1.19 as well (by the golang support policy, v1.19 is now no longer supported).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah; ok I can do that though I was a little hesitant as it would have implications on other go-repos
I can update those too as well though.
Update the default value for the go_version to only list non-EOL versions. Both 1.18.x and 1.19.x are no longer maintained. Additionally add 1.21.x
f2129fc
to
d82e36a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion for commit title:
config.yaml: add golang 1.21, remove golang 1.18 and 1.19
But LGTM as is too!
Thank you for the feedback; I think I prefer your commit message; but I also think the existing one was satisfactory and did not want to reset any approval. |
Related to this coreos/ignition#1733