-
Notifications
You must be signed in to change notification settings - Fork 440
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
Use golang 1.18 #786
Use golang 1.18 #786
Conversation
The build failed on: golangci/golangci-lint#2649 |
@@ -35,6 +35,7 @@ jobs: | |||
- name: Set up Go | |||
uses: actions/setup-go@v2 | |||
with: | |||
# TODO switch to Golang 1.18 once https://github.com/golangci/golangci-lint/issues/2649 is fixed | |||
go-version: 1.17 |
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 was able to fix the issue by using temporarily go 1.17 for lint.
once this PR is merged I will create an issue in this repo to track this.
@@ -1,6 +1,6 @@ | |||
module github.com/otel-allocator | |||
|
|||
go 1.17 | |||
go 1.18 |
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.
Isn't this the minimum version that is required to compile? Iam asking because of:
at RH there is probably no go 1.18 image for productization. We have to make sure the project can be built with go 1.17 as well
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 am able to build it with go 1.17 without issues (e.g. by changing go builder version in the dockerfile)
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.
oh ok, but it would allow people to use 1.18 features, right? Would then make sense to keep it at 1.17 until we have a build image?
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.
That is a valid argument. If somebody will use 1.18 features we will have to fork :/
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.
1.18 features make MUCH sense for Kubernetes Operators, like the support for generics. Much of the code can be simplified with that, I presume. But we are not in a hurry to change to it, though.
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.
LGTM!
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
PR rebased. With the latest version of the golang-ci our lint job passes fine without any workaround. |
LGTM! |
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay p.loffay@gmail.com
Replaces #784