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

Move setup process via script into container #2174

Merged
merged 17 commits into from Sep 11, 2021

Conversation

georglauterbach
Copy link
Member

Description

Decoupling setup process from setup.sh script by introducing a setup script inside the container that coordinates the setup process. This is not a breaking change. This way, we do not have to keep track of versions of setup.sh, which was said to be a no-no here. This is a blog post we're linking to in our documentation. This change brings the additional benefit for Kubernets users to be able to make use of setup now, without the need for setup.sh.

The maintainers discussion referencing / talking about the current setup.sh versioning-problem can be found here.

This PR was really made possible by @NorseGaud with #2134

Type of change

  • Improvement (non-breaking change that does improve existing functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@georglauterbach georglauterbach added pr/needs review priority/medium area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Sep 6, 2021
@georglauterbach georglauterbach added this to the v10.2.0 milestone Sep 6, 2021
@georglauterbach georglauterbach requested a review from a team September 6, 2021 14:50
@georglauterbach georglauterbach self-assigned this Sep 6, 2021
@georglauterbach
Copy link
Member Author

MkDocs does not seem to render ~~ (crossed out) correctly. @wernerfred @polarathene do you have any idea how to fix this? I mean, it's just a joke in the docs and we can remove it, but it makes me curious.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
georglauterbach and others added 3 commits September 7, 2021 12:49
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
polarathene
polarathene previously approved these changes Sep 9, 2021
@georglauterbach
Copy link
Member Author

@docker-mailserver/maintainers Tests should pass again (rebased on master with #2177 merged). Feel free to review again :)

Copy link
Member

@casperklein casperklein left a comment

Choose a reason for hiding this comment

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

See my suggestions to remove MacOS specific quirks, intruduced in #2001

target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
target/bin/setup Outdated Show resolved Hide resolved
Co-authored-by: Casper <casperklein@users.noreply.github.com>
georglauterbach and others added 4 commits September 11, 2021 18:08
Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
georglauterbach and others added 3 commits September 11, 2021 18:08
Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
@github-actions
Copy link
Contributor

Documentation preview for this PR is ready! 🎉

Built with commit: 512de25

@NorseGaud NorseGaud self-requested a review September 11, 2021 16:11
@polarathene polarathene merged commit c7e4981 into master Sep 11, 2021
@polarathene polarathene deleted the move-setup-into-container branch September 11, 2021 23:29
setup.sh Show resolved Hide resolved
NorseGaud pushed a commit to NorseGaud/docker-mailserver that referenced this pull request Sep 17, 2021
Decoupling setup process from `setup.sh` script by introducing a setup script _inside_ the container that coordinates the setup process.

**This is not a breaking change**. This way, we do not have to keep track of versions of `setup.sh`.

This change brings the additional benefit for Kubernetes users to be able to make use of `setup` now, without the need for `setup.sh`.

---

* move setup process into container; setup.sh versioning not needed anymore

* add tilde functionality to docs

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
NorseGaud pushed a commit to NorseGaud/docker-mailserver that referenced this pull request Sep 17, 2021
Decoupling setup process from `setup.sh` script by introducing a setup script _inside_ the container that coordinates the setup process.

**This is not a breaking change**. This way, we do not have to keep track of versions of `setup.sh`.

This change brings the additional benefit for Kubernetes users to be able to make use of `setup` now, without the need for `setup.sh`.

---

* move setup process into container; setup.sh versioning not needed anymore

* add tilde functionality to docs

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
@matrixes
Copy link
Contributor

I have updated the blog post to cover the new conditions regarding setup. I'd also like to take the opportunity to thank you for your efforts!

@georglauterbach
Copy link
Member Author

I have updated the blog post to cover the new conditions regarding setup. I'd also like to take the opportunity to thank you for your efforts!

You're welcome! :) You blog post was the reason I actually started this as it was very valid and hard (but correct, which I appreciate) criticism. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scripts kind/improvement Improve an existing feature, configuration file or the documentation priority/medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants