Skip to content
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

fix(charts/dex): Helm - Versioning #129

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mihaigalos
Copy link

@mihaigalos mihaigalos commented Apr 17, 2024

Overview

This PR adapts the versioning of the Helm chart to make it consistent with other projects and its internally used comments for versioning.

What this PR does / why we need it

Fixes: #125.

Special notes for your reviewer

Checklist

  • Change log updated in Chart.yaml (see the contributing guide for details)
  • Chart version bumped in Chart.yaml (see the contributing guide for details)
  • Documentation regenerated by running make docs

Signed-off-by: Mihai Galos <mihai.galos@rbinternational.com>
Signed-off-by: Mihai Galos <mihai.galos@rbinternational.com>
Signed-off-by: Mihai Galos <mihai.galos@rbinternational.com>
@mihaigalos
Copy link
Author

mihaigalos commented Apr 17, 2024

Dear maintainers. Does the chart automatically get released to gh-pages?
I assume yes, but in the end, what I'm after is this here:

-    appVersion: 2.39.1
+    appVersion: v2.39.2

charts/dex/Chart.yaml Outdated Show resolved Hide resolved
Signed-off-by: Mihai Galos <mihai.galos@rbinternational.com>
@mihaigalos
Copy link
Author

Hi @sagikazarmark, is this now ready?

Signed-off-by: Mihai Galos <mihai@galos.one>
@mihaigalos
Copy link
Author

Up @sagikazarmark - any news here?

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't fix #125

The v prefix should be added to appVersion.

Prefixing also needs to be removed from here: https://github.com/dexidp/helm-charts/blob/master/charts/dex/templates/deployment.yaml#L59

Finally, this change should result in a minor chart version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Inconsistent image tagging in Helm chart appVersion
2 participants