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

Merge FunkyPenguin's Changes #89

Merged
merged 25 commits into from
Jan 29, 2024
Merged

Merge FunkyPenguin's Changes #89

merged 25 commits into from
Jan 29, 2024

Conversation

cfis
Copy link
Collaborator

@cfis cfis commented Jan 10, 2024

This MR rebases funkypenguin's changes (see https://github.com/funkypenguin/helm-docker-mailserver) to the master branch. See #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 ^^^

Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
Signed-off-by: David Young <[email protected]>
@cfis cfis requested a review from funkypenguin as a code owner January 10, 2024 01:08
@cfis cfis mentioned this pull request Jan 10, 2024
@cfis cfis changed the title Penguin Merge FunkyPenguin's Changes Jan 10, 2024
@funkypenguin
Copy link
Contributor

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

@funkypenguin
Copy link
Contributor

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

@cfis
Copy link
Collaborator Author

cfis commented Jan 10, 2024

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
Copy link
Collaborator Author

cfis commented Jan 13, 2024

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
Copy link
Collaborator Author

cfis commented Jan 25, 2024

@georglauterbach
Copy link
Member

georglauterbach commented Jan 25, 2024

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
Copy link
Collaborator Author

cfis commented Jan 25, 2024

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

@cfis
Copy link
Collaborator Author

cfis commented Jan 26, 2024

Linting should be fixed (tested locally).

@georglauterbach
Copy link
Member

georglauterbach commented Jan 26, 2024

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 🚀

…ove cert-manager certificate because this should be setup *before* docker mailserver is installed.
@cfis cfis merged commit 051d267 into docker-mailserver:master Jan 29, 2024
4 checks passed
@cfis cfis mentioned this pull request Feb 1, 2024
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.

4 participants