Restore v prefix in app configmap #2129
Conversation
| @@ -44,7 +44,7 @@ data: | |||
| { | |||
| "kubeappsCluster": "{{ template "kubeapps.kubeappsCluster" . -}}", | |||
| "kubeappsNamespace": "{{ .Release.Namespace }}", | |||
| "appVersion": "{{ .Chart.AppVersion }}", | |||
| "appVersion": "v{{ .Chart.AppVersion }}", | |||
There was a problem hiding this comment.
AIUI, in #2123 we removed the v in chart_sync_utils.sh from the Chart.yaml and at the same time updated AppListItem.tsx so that a double-v won't be displayed either way. In this PR we're adding the v back in but to the dashboard config so that links to docs in GH work etc., so that there's now two places affecting this.
Why not instead revert the change to chart_sync_utils.sh rather than having different appVersions in our chart and dashboard code?
There was a problem hiding this comment.
The thing is that we want (at least IMO) the v in the tag and in the release name because it's the convention that we have been following (and looks better?) but in the Chart.yaml, that version should be semver compatible if possible (so no v there). That's what causes the confusion.
There was a problem hiding this comment.
From https://helm.sh/docs/topics/charts/#the-chartyaml-file it's an optional field and not required to be semver: "appVersion: The version of the app that this contains (optional). This needn't be SemVer."
Or are we using it somewhere else requiring it to be semver? Perhaps the pipeline? Sure, if all bitnami charts use semver for appVersion, +1 to do so even though it's not required. I just thought the original PR was simply about the double v in the UI.
There was a problem hiding this comment.
Or are we using it somewhere else requiring it to be semver? Perhaps the pipeline? Sure, if all bitnami charts use semver for appVersion
Yes it's not required but a nice thing to do. Also the bitnami charts all follow semver for the appVersion.
I just thought the original PR was simply about the double
vin the UI.
Yes, when I was working on fixing the double v I re-thought the v in the appVersion field. These are two different things.
Description of the change
After #2123, we have removed the
vprefix from the chart appVersion. The problem is that we are using that version to generate some links to the documentation (e.g.href={https://github.com/kubeapps/kubeapps/blob/${appVersion}/docs/user/getting-started.md}) so we need to match the GitHub tag there.