[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: Greg Stein <gstein_at_gmail.com>
Date: Wed, 21 Jan 2009 13:27:33 +0100

On Wed, Jan 21, 2009 at 12:43, Jon Bendtsen <jbendtsen_at_laerdal.dk> wrote:
> On 21/01/2009, at 12.13, Greg Stein wrote:
>...
>> Rather than applying specific names to the components, how about a
>> more general solution such as (for your example above):
>>
>> path-headers = module:0 branch:1 submodule:2 mysubsub:3 file:-1
>
> Thats already possible. You dont have to get all 4, for each of the 4
> you can
> choose to get them or not. Though the header name will be like those
> names
> i choose when i made it to fit our usage. I just figured that others
> would like
> the feature.

I agree with the feature, but am saying that the header names will be
"wrong". If I have a repository, like Subversion's own, that has:

  /trunk
    /tools
    /subversion
  /tags
    /1.5.0
    /1.5.1
  /branches
    /1.5.x
    /some-issue

Then I could end up with:

  X-module: trunk
  X-branch: subversion

or:

  X-module: branches
  X-branch: 1.5.x

The first is broken, the second has a good X-branch header, but the
X-module is nonsense.

So... I'm suggesting a more flexible way to parse the path and to
assign names to each of the segments of the path.

>> Or something like that. Basically: NAME:SEGMENT-NUMBER. Of course, the
>> problem here is that "submodule" is sometimes at segment 1 of the path
>> (under /trunk), but at segment 2 for /tags and /branches.
>>
>> The difference between /trunk and /tags and /branches could be handled
>> with a separate [group] which matches specific paths using for_paths.
>
> The patch can have individual group configuration. However, to get
> emails
> for a new branch by using a new group you have to write a new group into
> the configuration file. We dont have to do that, because if someone
> makes
> a new branch, then the name will be put into the header of the email
> automatically.

What I mean is something like this:

[trunk-commits]
for_paths = trunk/
path-headers = module:1

[branch-commits]
for_paths = branches/
path-headers = branch:1 module:2

[tag-commits]
for_paths = tags/
path-headers = tag:1 module:2

Email for changes in trunk would generate X-module with the segment
just after trunk/. Changes in tags and branches would skip segment 0
("tags" or "branches") and define new X-tag or X-branch headers, along
with the proper segment for the X-module header.

>...
>> if we follow the above suggestion and create a mechanism allowing the
>> configuration to specify name/value, then maybe we would pass some
>> kind of a HeaderSpec object (to be designed).
>
> I see your point of passing 5 seperate parameters, but someone reading
> the code can understand what this parameter does, and i was expecting
> everyone to always use all available headers, so i figured i should just
> pass them all along.

It took a while for me to figure out you were just bundling 5 values
into one parameter, then immediately unbundling them. The
"header_list" parameter has no other semantic than as a bundle. To
take that pattern to its illogical conclusion, all methods would take
a single parameter, all callers would bundle up a list, and the
function would unbundle it first thing.

Again, if we have a generalized mechanism for tearing apart paths,
that definition could go into a single object and passed that way.
We'd have the flexibility that any project would need, but would also
be able to pass a single parameter rather than five.

>...
>>> ...
>>> class SMTPOutput(MailedOutput):
>>> "Deliver a mail message to an MTA using SMTP."
>>>
>>> - def start(self, group, params):
>>> + def start(self, group, params, header_list):
>>> MailedOutput.start(self, group, params)
>>
>> header_list should be passed to the superclass.
>
> Why? Only the emails use it

(!!)

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.

>...
>> 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.

>...
> That may be so, but author only started showing up when i did group and
> the other headers. I certainly dont remember seeing author before, and i
> didnt make changes to the underlying SMTP system. But i maybe just have
> overlooked the author field. But why would the SMTP system put in an
> author
> field? wouldnt it just put in an from address?

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)

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1041089
Received on 2009-01-21 13:27:49 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.