[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email

From: Jon Bendtsen <jbendtsen_at_laerdal.dk>
Date: Thu, 22 Jan 2009 12:20:19 +0100

On 21/01/2009, at 15.03, Greg Stein wrote:

> On Wed, Jan 21, 2009 at 13:46, Jon Bendtsen <jbendtsen_at_laerdal.dk>
> wrote:

[cuuuuuuuuuut]

>>> Because you defined the superclass to take that parameter. The
>>> MailedOutput.start() call is going to throw an exception because you
>>> don't pass the value.
>>
>> I didnt experience exceptions or other faults on the ~25.000 changes
>> we had in our subversion repository. I started the script by "hand"
>> to
>> a dummy email address, and there was nothing on stdout or stderr.
>
> The exception would be raised if you use the smtp_hostname
> configuration option. I suspect you maybe used the mail_command
> configuration instead? Or maybe left both blank and generated to
> stdout instead?

Fixed in the attached diff

>>>>> What is this stuff with [x] at the beginning? I see no doc about
>>>>> it,
>>>>> or other references/use.
>>>>
>>>> what [x]? i do not understand. I stole the if_then_else from some
>>>> other
>>>> part of the script. I think it was where the group setup the from
>>>> address.
>>>
>>> Oh. I see it now in other areas of the code. Something new that was
>>> added when I wasn't looking. Ugh.
>>
>> I deliberately copied rather than code it myself.
>
> Yup. Not your fault at all!
>
> Now that I have found that stuff and figured out what it is for... it
> doesn't apply in this case. Being able to replace the "split
> character" is important for cases where whitespace can occur within
> the values. That will not be the case for your header logic. So we can
> keep it very simple with just a .split() and no testing for [.].

Lets wait with this until we agree the path setup ;-)

>>> No idea. But the point is that I don't think that comment should be
>>> there, since we don't actually add that field anywhere. (we *do* put
>>> "Author:" into the body of the message, however)
>>
>> hmm, maybe it was a missing \n that cause it to show up in the
>> header.
>> In the beginning my users requested an X-author: field, but when i
>> saw
>> author: i figured i didnt have to add it.
>
> Ah! Quite possible.
>
> And yes, an X-author header should be quite easy to add. It would be
> handy if all commit email comes from the same From: address (such as
> codesite-noreply_at_google.com on Google Code).

It was a missing \n i just tested with the latest mailer.py from trunk

Also fixed in the attached diff, meaning i added a \n and an X-author
field

JonB

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1043107

On 21/01/2009, at 15.03, Greg Stein wrote:

> On Wed, Jan 21, 2009 at 13:46, Jon Bendtsen <jbendtsen_at_laerdal.dk>
> wrote:

[cuuuuuuuuuut]

>>> Because you defined the superclass to take that parameter. The
>>> MailedOutput.start() call is going to throw an exception because you
>>> don't pass the value.
>>
>> I didnt experience exceptions or other faults on the ~25.000 changes
>> we had in our subversion repository. I started the script by "hand"
>> to
>> a dummy email address, and there was nothing on stdout or stderr.
>
> The exception would be raised if you use the smtp_hostname
> configuration option. I suspect you maybe used the mail_command
> configuration instead? Or maybe left both blank and generated to
> stdout instead?

Fixed in the attached diff

>>>>> What is this stuff with [x] at the beginning? I see no doc about
>>>>> it,
>>>>> or other references/use.
>>>>
>>>> what [x]? i do not understand. I stole the if_then_else from some
>>>> other
>>>> part of the script. I think it was where the group setup the from
>>>> address.
>>>
>>> Oh. I see it now in other areas of the code. Something new that was
>>> added when I wasn't looking. Ugh.
>>
>> I deliberately copied rather than code it myself.
>
> Yup. Not your fault at all!
>
> Now that I have found that stuff and figured out what it is for... it
> doesn't apply in this case. Being able to replace the "split
> character" is important for cases where whitespace can occur within
> the values. That will not be the case for your header logic. So we can
> keep it very simple with just a .split() and no testing for [.].

Lets wait with this until we agree the path setup ;-)

>>> No idea. But the point is that I don't think that comment should be
>>> there, since we don't actually add that field anywhere. (we *do* put
>>> "Author:" into the body of the message, however)
>>
>> hmm, maybe it was a missing \n that cause it to show up in the
>> header.
>> In the beginning my users requested an X-author: field, but when i
>> saw
>> author: i figured i didnt have to add it.
>
> Ah! Quite possible.
>
> And yes, an X-author header should be quite easy to add. It would be
> handy if all commit email comes from the same From: address (such as
> codesite-noreply_at_google.com on Google Code).

It was a missing \n i just tested with the latest mailer.py from trunk

Also fixed in the attached diff, meaning i added a \n and an X-author
field

JonB

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1043107

Received on 2009-01-22 13:08:18 CET

This is an archived mail posted to the Subversion Dev mailing list.