-
Notifications
You must be signed in to change notification settings - Fork 23
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 go to 1.23 #57
Conversation
bcc3d78
to
b8f711d
Compare
@SANJEEV-Choubey Please can you review |
Dockerfile
Outdated
@@ -1,4 +1,4 @@ | |||
FROM golang:1.19-alpine as builder | |||
FROM golang:1.21-alpine as builder |
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.
It would be great if you could upgrade go version to 1.23
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.
Done
go.mod
Outdated
@@ -1,19 +1,19 @@ | |||
module github.com/IBM-Cloud/redli | |||
|
|||
go 1.19 | |||
go 1.21 |
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.
It would be great if you could upgrade go version to 1.23
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.
Done
@ade18 Thank you very much for your time and for creating this PR.Could below two things will be taken care?
|
b8f711d
to
7a6df6b
Compare
7a6df6b
to
87d6db0
Compare
Thank you for reviewing, i have made the changes. @SANJEEV-Choubey |
Could you also get rid of vendor and start using the module? |
|
@ade18 As we use go mod so now go/vendor is not required, As you are updating go version it would be great we should only use modules in this PR otherwise a operate PR will be needed to remove vendor. Action Item:- Delete vendor directory from this repo and Build and test it if it is working fine please update the PR and upload a test evidence. |
@SANJEEV-Choubey the vendor directory is deleted in this PR thats why there is over 300+ files appearing being removed from the repository including the directory itself. You can navidate to my forked repo/branch and will see its indeed removed as shown in the PR. See evidences Compiling the code ![]() See image being built: ![]() |
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
Update go to 1.21 to remediate crypto/tls vulnerability