On 21/01/2009, at 13.27, Greg Stein wrote:
> 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.
Yes, but that is just purely cosmetic at and what you actually have to
program your email client to sort and filter by. In our subversion to
filter
and sort on branch, you would have to use the X-branch: header, and
your subversion would have to use the X-module: header. It is still
possible
to do the filter and sort. But maybe confusing to some users.
>>> 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.
i understand now.
>> ...
>>> 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.
okay, i see your point
>
>> ...
>>>> ...
>>>> 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.
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.
>
>> ...
>>> 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.
>
>> ...
>> 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)
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.
JonB
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1041124
Received on 2009-01-21 14:44:25 CET