search

docker-mailserver / docker-mailserver-helm

Kubernetes Helm chart for docker-mailserver
https://github.com/docker-mailserver/docker-mailserver/
MIT License
107 stars 67 forks source link

Merge FunkyPenguin's Changes #89

Closed cfis closed 9 months ago

cfis commented 10 months ago

This MR rebases funkypenguin's changes (see https://github.com/funkypenguin/helm-docker-mailserver) to the master branch. See https://github.com/docker-mailserver/docker-mailserver-helm/issues/88.

Although there are a lot of commits, in the end only 4 files were changed. The most significant difference is using uppercase ENV variable names that match the names in https://docker-mailserver.github.io/docker-mailserver/latest/config/environment/ (so that seems like a good idea to me).

@funkypenguin ^^^

funkypenguin commented 10 months ago

Sorry for the delay @cfis, and thank you for persisting here!

funkypenguin commented 10 months ago

The chart wants a semver version bump (in Chart.yaml) before linting will pass it :)

cfis commented 10 months ago

Done. I also went ahead and updated the appVersion to 13.2.0 as well as the dockermailer image tag.

For the chart version I set it to 2.2.0, but I can see an argument to 3.0.0 due to the changes in the ENV variables. Let me know what you prefer.

And no worries, you are the one who did all the work!

cfis commented 10 months ago

Hi @funkypenguin - just wanted to follow up on this MR.

Also, I have a some more significant updates to the repo which I would love to merge in (see https://github.com/cfis/docker-mailserver-helm/commits/master/). Not sure if you have time to review them, or if someone else could? Or I'm happy to merge in if you allow me write access.

Thanks - Charlie!

cfis commented 10 months ago

Fyi - https://github.com/docker-mailserver/docker-mailserver/issues/3825

georglauterbach commented 10 months ago

Linting seems to fail still; can and will this be fixed? The changes look good!

@cfis we lack maintainers for the chart. Are you interested in joining the maintainer's team for the chart repository? This way, you can delelop on this more easily and merge smaller PRs yourself. Moreover, you could review and merge PRs from other contributors.

cfis commented 10 months ago

Yes, will fix linting. And happy to join maintainers list - thanks!

cfis commented 10 months ago

Linting should be fixed (tested locally).

georglauterbach commented 10 months ago

I sent you an invitation @cfis. When you accept, you should be able to merge this PR, as I will approve this PR in a second. What would be ideal is a fix of the CI (chart-testing seems to be broken) and merging the now-stale PRs in this repository :) Feel free to tackle these tasks 🚀