Remove v prefix from appVersion #279
Conversation
|
Ha. On #277 (comment) I wrote:
So.. you're saying we already do this:
OK. Where is that logic? And while answering this question we found that it's not there: Was removed here: Edit: Only now I look at the patch and see that you have re-added that logic here. Ha! :) |
| # Note(JP): this currently defines the default `tag` value in a k8s | ||
| # image specification, and therefore must currently be v-prefixed. | ||
| appVersion: "v25.3.0-rc.1" | ||
| # image specification. A "v" is prefixed in our template helpers to ensure constency. |
There was a problem hiding this comment.
"consistency".
can do after merge.
There was a problem hiding this comment.
Can we change the comment to say:
This is used to construct the default
tagvalue for the k8s
image specification (with a "v" is prefixed to this value), see
_helpers.tpl.
There was a problem hiding this comment.
Done now since we have the issue below.
| else | ||
| VERSION=$1 | ||
| fi | ||
| # Remove any v prefix, if exists. |
There was a problem hiding this comment.
But.. the code below is broken. :)
VERSION="${VERSION#v}"
elezar
force-pushed
the
make-app-version-consistent
branch
from
e6470e3 to
ce0b144
Compare
jgehrcke
left a comment
jgehrcke
left a comment
There was a problem hiding this comment.
To add to the v-prefix frustration: the "Remove any v prefix, if exists" logic must again be amended in the current patch.
This change removes the v prefix from the Helm chart appVersion. The prefix is prepended in the template helpers if required. This aligns this project with other projects such as the device plugin. Signed-off-by: Evan Lezar <[email protected]>
elezar
force-pushed
the
make-app-version-consistent
branch
from
ce0b144 to
91555a9
Compare
jgehrcke
left a comment
jgehrcke
left a comment
There was a problem hiding this comment.
❤️
No more v prefixes for me for 10 years. Hello!
| if [ -z "$1" ]; then | ||
| VERSION=$(awk -F= '/^VERSION/ { print $2 }' versions.mk | tr -d '[:space:]') | ||
| # Remove any v prefix, if exists. | ||
| VERSION="${VERSION#v}" |
There was a problem hiding this comment.
Btw, all of this is a great example that (well-maintained) code comments easily can save a bunch of time overall.
| */}} | ||
| {{- define "nvidia-dra-driver-gpu.fullimage" -}} | ||
| {{- $tag := printf "%s" .Chart.AppVersion }} | ||
| {{- $tag := printf "v%s" .Chart.AppVersion }} |
There was a problem hiding this comment.
In the future we should add a code comment here, too, if the template language allows for that.
github-project-automation
bot
moved this to Backlog
in Planning Board: k8s-dra-driver-gpu
2 participants
This change removes the v prefix from the Helm chart appVersion. The prefix is prepended in the template helpers if required.
This aligns this project with other projects such as the device plugin.