make SendmailModule configurable

[hos-ftv]

fixes #34

I've added a switch to completely disable writing the smtp-settings to the Config.pm which allows us to configure those settings via SysConf.

P.S. Oh, btw. Feel free to edit anything you like if there are any mistakes (i.e. spelling, prefered syntax).

[hos-ftv]

May this also addresses the issue which raised pull request #55

[juanluisbaptiste]

Hi,

Thanks for the PR. I think we are mixing two issues here that I would rather work separated. One is making SMTP configuration configurable by env vars and the other one is about how to do it, if via Config.pm or sysconf.

This PR should be addressing only the first issue, which is the topic of issue #34 that this PR addresses.

The disabling of this configuration via Config.pm could be implemented to make it more general to enable/disable Config.pm configuration entirely instead of just the SMTP settings. Also I think we could achieve the same thing by making all the SMTP_ vars optional, as you made the new username and password ones. If you don't set them then Config.pm will not be modified with those parameters.

So, about the changes about making SendmailModule configurable, I think they are ok, so if you agree, please remove from this PR the CONFIGURE_MAIL_VIA_SYSCONF check and we are good to go. You can open another issue if you want to discuss it.

[hos-ftv]

I totally agree that enabling/disabling configuration via Config.pm could be handled in a separate PR. I'll extract that and keep the idea of a global switch in mind.

Making all the SMTP_ - vars optional was my first intention but this could/will break existing setups using juanluisbaptiste/docker-postfix (or any similar) which rely on SendmailModule, SendmailModule::Host and SendmailModule::Port are being set to their current default values. So I discarded this idea. If you're ok with this change I'd like to harmonize this block as you've mentioned.

Moreover, I'm not quite sure if configuring SendmailModule the way it is implemented in this (short) way is a good idea or if it would be better to pass in the whole module-name/-string. So instead of just only SMTP Kernel::System::Email::SMTP could be required.

[juanluisbaptiste]

Making all the SMTP_ - vars optional was my first intention but this could/will break existing setups using juanluisbaptiste/docker-postfix (or any similar) which rely on SendmailModule, SendmailModule::Host and SendmailModule::Port are being set to their current default values.

Sorry for the delay in answering. Yes you make a point about breaking current configurations, I'll think a little bit more if there's a better way to handle this, or maybe adding a notice at bot that this changed or is going to, could be how to handle it.

[hos-ftv]

Hi @juanluisbaptiste after thinking that over it does not seem to break current configurations as non of the methods in functions.sh will remove any entry from the Config.pm. So existing setups have to be "cleaned up" manually before the new configuration takes effect.

[juanluisbaptiste]

Hi @hos-ftv, sorry for the late reply. I think too that as you say, while we don't remove current smtp configuration options and we handle default values we will not break current installs. Please do a clean commit with this changes, as this PR has multiple outdated commits .

Thanks.

[hos-ftv]

so I'll close this one