Login | Register
My pages Projects Community openCollabNet

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

subversion
Discussion topic

Back to topic list

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

Author gstein
Full name Greg Stein
Date 2009-01-21 04:27:35 PST
Message On Wed, Jan 21, 2009 at 12:43, Jon Bendtsen <jbendtsen at laerdal dot 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

« Previous message in topic | 7 of 25 | Next message in topic »

Messages

Show all messages in topic

[PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-16 06:21:11 PST
     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email arfrever Arfrever Frehtes Taifersar Arahesis 2009-01-16 12:10:33 PST
         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-16 13:18:50 PST
             Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-19 07:37:54 PST
                 Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-21 03:13:35 PST
                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-21 03:43:36 PST
                         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-21 04:27:35 PST
                             Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-21 04:47:32 PST
                                 Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-21 06:03:54 PST
                                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-22 02:03:55 PST
                                         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-22 02:30:37 PST
                                             Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-22 02:36:47 PST
                                                 Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-22 03:52:00 PST
                                                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-22 04:15:34 PST
                                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-22 03:20:25 PST
                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-21 05:04:45 PST
                         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-21 05:30:28 PST
                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-26 05:02:57 PST
                         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gstein Greg Stein 2009-01-26 06:33:20 PST
                             Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-01-27 03:26:37 PST
                                 Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-02-04 08:50:06 PST
                                 Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-06-25 09:34:51 PDT
                                     Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-06-26 06:33:14 PDT
                                         Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email gavinbaumanis Gavin Baumanis 2009-07-21 06:00:03 PDT
                             Re: [PATCH] Issue 3227 - adding some extra "custom" headers to the email jbendtsen Jon Bendtsen 2009-02-23 07:48:58 PST
Messages per page: