Skip to content

Conversation

@mhlgio
Copy link
Contributor

@mhlgio mhlgio commented Jul 5, 2025

Before introducing other changes, this PR updates the subchart common to the current version.

BREAKING CHANGE: common 1.5.1 -> 4.1.2, which had introduced some breaking changes between the chart's values.

fixes #6

@mhlgio mhlgio force-pushed the add-app-template branch from 0a2b0e1 to 1b2e6a7 Compare July 5, 2025 22:21
@mhlgio mhlgio force-pushed the add-app-template branch from 1b2e6a7 to 970ba1c Compare July 6, 2025 04:58
NAMESPACE=$(RELEASE_NAME)
OUTPUT_DIR=output

template: values.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Would we need to run that in ci during release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, this is for dev purpose, I was using that a lot during development.

I'm not THAT familiar with writing Helm charts, but it seems that the common team have some pattern for unit testing the charts, might look into that later.

Copy link
Member

Choose a reason for hiding this comment

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

You might not need to bother with Typesense since we're currently exploring if we'll get rid of it completely (we'll need an alternative to move forward with that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I'm targeting 0.24.6 for now. Trying to establish parity between the charts as a baseline.

For 3.x release of the chart I would target getting rid of the commons. It seems good as a starting point, but in a longer term it feels like I'm trying to understand how to work with the library instead of how to deploy the chart.

{{- if .Values.redis.enabled }}
VIKUNJA_REDIS_ENABLED: true
{{- end }}
{{- if .Values.redis.enabled }}
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes :)

that's why it's still in a draft, it's generating mostly similar resources but there are still some things missing (e.g. typesense). it seems to me that the naming patterns changed too.

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.

Bump the common chart dependency

2 participants