Fix kubectl-retina version
#1023
Conversation
@microsoft-github-policy-service agree |
|
I think the issue lies with the passing of the Currently, my approach involves using logging to verify if If possible, could you enable CI testing for this? @nddq |
cli/Dockerfile
Outdated
| @@ -15,6 +15,9 @@ ENV GOOS=${GOOS} | |||
| ARG GOARCH=amd64 | |||
| ENV GOARCH=${GOARCH} | |||
|
|
|||
| # Debugging: Print the values of VERSION and APP_INSIGHTS_ID | |||
| RUN echo "VERSION=${VERSION}" && echo "APP_INSIGHTS_ID=${APP_INSIGHTS_ID}" | |||
There was a problem hiding this comment.
Maybe remove the debug lines before merging?
There was a problem hiding this comment.
+1 . We should remove the debug lines.
cli/cmd/version.go
Outdated
| @@ -11,11 +11,20 @@ import ( | |||
| "github.com/microsoft/retina/internal/buildinfo" | |||
| ) | |||
|
|
|||
| // This variable is used by the "version" command and is set during build. | |||
| // Default to a safe value if not set. | |||
| var defaultVersion = "v0.0.5" | |||
There was a problem hiding this comment.
Wouldn't it be a misleading info? I don't think it makes sense to have a "default" version
There was a problem hiding this comment.
Yeah, I'd almost rather an empty string here and a loud notification that this wasn't set by some pipeline.
cli/Dockerfile
Outdated
| @@ -15,6 +15,9 @@ ENV GOOS=${GOOS} | |||
| ARG GOARCH=amd64 | |||
| ENV GOARCH=${GOARCH} | |||
|
|
|||
| # Debugging: Print the values of VERSION and APP_INSIGHTS_ID | |||
| RUN echo "VERSION=${VERSION}" && echo "APP_INSIGHTS_ID=${APP_INSIGHTS_ID}" | |||
There was a problem hiding this comment.
+1 . We should remove the debug lines.
cli/cmd/version.go
Outdated
| fmt.Println("buildinfo.Version is not set successfully, showing default version:") | ||
| fmt.Println(defaultVersion) |
There was a problem hiding this comment.
I think you can probably get a more useful "default" value out of https://pkg.go.dev/runtime/debug#BuildInfo . There's a property in there for the vcs.revision which should be the commit the binary was built with. It won't be the tag, but the commit is plenty useful (and would be handy for development builds).
cli/cmd/version.go
Outdated
| @@ -11,11 +11,20 @@ import ( | |||
| "github.com/microsoft/retina/internal/buildinfo" | |||
| ) | |||
|
|
|||
| // This variable is used by the "version" command and is set during build. | |||
| // Default to a safe value if not set. | |||
| var defaultVersion = "v0.0.5" | |||
There was a problem hiding this comment.
Yeah, I'd almost rather an empty string here and a loud notification that this wasn't set by some pipeline.
go.mod
Outdated
| @@ -22,7 +22,7 @@ retract ( | |||
|
|
|||
| require ( | |||
| cel.dev/expr v0.15.0 // indirect | |||
| code.cloudfoundry.org/clock v1.0.0 // indirect | |||
| code.cloudfoundry.org/clock v1.21.0 // indirect | |||
There was a problem hiding this comment.
I'd drop the go.mod and go.sum changes from this. We're not changing any dependencies here, and the upgrades should be handled by dependabot. That'll make the PR simpler.
Description
Since
v0.0.17,kubectl-retina versionwill only show a empty line, demonstrated in #1013Related Issue
#1013
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
None
Additional Notes
None
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.