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

refactor: move 'image' properties up in the values.yaml file #849

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xakraz
Copy link

@xakraz xakraz commented Feb 25, 2025

Changes

  • Update the image name and tag in the deployment.yaml file
  • Add a new section for image details in the values.yaml file with repository, tag, and pull policy
  • Remove the image section from the reloader block in the values.yaml file, since the image details were moved to a new image section
  • Update the version number in the Chart.yaml file from 1.3.0 to 1.3.1

Motivations

We use Helmper to cache public Helm charts and related container images.

Helmper is not able to identify the image properties from this chart: https://christoffernissen.github.io/helmper/docs/parser

We thought it would be easier to submit a PR for updating this chart and make it like many other popular charts rather than updating Helmper 😇

@MuneebAijaz
Copy link
Contributor

@xakraz thanks for the PR, we will review it soon. On a side note, dont you think it should be major bump instead of a patch, since image placement change would be a breaking change?

@xakraz
Copy link
Author

xakraz commented Feb 25, 2025

@MuneebAijaz , I wondered about the version bump.
I can do a major one. I will update my PR tomorrow (CET).

Thanks for considering it 🙏

@karl-johan-grahn
Copy link
Contributor

Yes do a major bump instead

- Update the version number in the `Chart.yaml` file from `1.3.0` to `1.3.1`
- Update the image name and tag in the `deployment.yaml` file
- Add a new section for image details in the `values.yaml` file with repository, tag, and pull policy
- Remove the `image` section from the `reloader` block in the `values.yaml` file, since the image details were moved to a new image section
- Modify the `containerSecurityContext` section in the `deployment.yaml` file
@xakraz
Copy link
Author

xakraz commented Feb 26, 2025

Thanks @karl-johan-grahn for the review 🙏🏻
I have updated the Chart version as requested.

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.

3 participants