[PATCH 1 of 5] e-mail: describe and clarify e-mail settings in the ini file

[Thomas De Schampheleire]

On Mon, Jul 13, 2015 at 8:19 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 07/13/2015 10:45 AM, Thomas De Schampheleire wrote:
>>
>> # HG changeset patch
>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>> # Date 1436644666 -7200
>> #      Sat Jul 11 21:57:46 2015 +0200
>> # Node ID c22a219636b830bac4b6125b306f6c8506f81d99
>> # Parent  4a3708db60aef76020f4fefacf861ee830abc564
>> e-mail: describe and clarify e-mail settings in the ini file
>>
>> Add some description of the e-mail related settings in the ini file.
>> Use sensible (uncommented) defaults for app_email_from and email_prefix.
>
>
> I wonder, why are these new values more sensible?

In configuration files for a piece of software, it is common practice
that key/value lines that are commented out show the default value.
However, for the Kallithea config files it is not at all true. For
example: the subject prefix looks as defaulting to [Kallithea], but
the code itself defaults to an empty prefix.

So, I think the defaults in the ini file and the defaults in the code
should line up.

See below for details.

>
>> Clarify that the default value of smtp_use_ssl is false by updating the
>> commented setting.
>>
>> This also deletes the commented setting of 'error_message' which does not
>> seem to be used. It is referring to the error_message variable in Paste,
>> which has as description (Paste:ErrorMiddleware):
>>      When debug mode is off, the error message to show to users.
>> However, setting this value apparently made no effect at all in Kallithea.
>
>
> It would be nice if such related but separate spin-off changes could be done
> in separate changesets.

Ok, will do.

>
> It would also be nice if the .ini files could be updated with the templates
> so the diff remains as small as possible.

Ah, yes, I forgot that there were also real .ini files in the
Mercurial tree. But, what is their purpose, really? Should we really
keep them?

>
>> diff --git a/kallithea/bin/template.ini.mako
>> b/kallithea/bin/template.ini.mako
>> --- a/kallithea/bin/template.ini.mako
>> +++ b/kallithea/bin/template.ini.mako
>> @@ -10,22 +10,29 @@ debug = true
>>   pdebug = false
>>   <%text>
>>
>> ################################################################################
>> -## Uncomment and replace with the address which should receive
>> ##
>> -## any error reports after application crash
>> ##
>> -## Additionally those settings will be used by Kallithea mailing system
>> ##
>> +## E-mail settings
>> ##
>> +##
>> ##
>> +## email_to: The e-mail address to send error reports to.
>> ##
>
>
> I think it would be better if the description could be rephrased to answer:
>
> Who will send which error reports ... and when?
>
> The texts in kallithea/templates/admin/settings/settings_email.html (and
> perhaps some code comments) should perhaps be synchronized and clarified
> while we are at it.

I'll look into it.

>
>> +## error_email_from: The sender of error e-mails.
>> ##
>> +## app_email_from: The sender of mails originating from Kallithea, for
>> ##
>> +##                 example to notify users about new comments, pull
>> requests, ##
>> +##                 etc.
>> ##
>> +## email_prefix: The subject prefix for mails originating from Kallithea.
>> ##
>> +##
>> ##
>> +## Note: your SMTP server may require app_email_from/error_email_from to
>> be   ##
>> +## valid, existing addresses.
>> ##
>>
>> ################################################################################</%text>
>>   #email_to = admin at localhost
>> -#error_email_from = paste_error at localhost
>> -#app_email_from = kallithea-noreply at localhost
>> -#error_message =
>> -#email_prefix = [Kallithea]
>> +#error_email_from = kallithea-noreply at localhost
>> +app_email_from = kallithea-noreply at localhost
>
>
> Why set app_email_from but not error_email_from?

I think my reasoning was that it didn't make sense to set
error_email_from when email_to is not set (and for which we cannot
provide a reasonable default). But maybe that reasoning is bogus and
error_email_from can be set just as well.

>
> And is "@localhost" really a default value that is expected to be correct
> and work in most cases? I don't think so. The System itself will probably be
> better at coming up with a @hostname address.

The default app_email_from is 'Kallithea', so not even an e-mail
address. Having an @localhost address seems already better to me, even
though a real admin will probably want to change it. Still, for cases
where the SMTP server doesn't really care about the from address, the
@localhost address should work fine.

>
>> +email_prefix = [Kallithea]

>
>
> Is it really an improvement to enable this by default? And should it tag
> everything with [Kallithea]? This is information that follows from the
> sender ... and a user might be using many different code hosting sites where
> it not really is important which of them use Kallithea.

As mentioned, the default suggested by the ini file does not match
with the actual default.
The subject prefix '[Kallithea]' seemed sensible to me as default, but
empty is fine for me too if you prefer. In fact, the latter is also
what we have set in production.

/Thomas