On Mon, Jan 31, 2011 at 10:52 PM, Noorul Islam K M <noorul_at_collab.net> wrote:
> Hyrum K Wright <hyrum_at_hyrumwright.org> writes:
>
>> On Mon, Jan 31, 2011 at 10:53 AM, Noorul Islam K M <noorul_at_collab.net> wrote:
>>
>>> Daniel Becroft <djcbecroft_at_gmail.com> writes:
>>>
>>>> On 27/01/2011, at 17:04, Noorul Islam K M <noorul_at_collab.net> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I am planning to work on issue 3690. Before starting with this I have
>>>>> few questions.
>>>>>
>>>>> 1. Hyrum updated the issue with his comment stating that already there
>>>>> is work going on in the branch ignore-mergeinfo which addresses
>>>>> subset of issue 3690, i.e ignoring changes to svn:mergeinfo. Is
>>>>> svn:mergeinfo an svn property set using svn pset command? I think Zvi
>>>>> Rackover is talking about a new option passing which a user can
>>>>> ignore revisions with just the following property changes alone.
>>>>>
>>>>> author eol-style externals keywords mime-type
>>>>> date executable ignore log needs-lock
>>>>
>>>> Fyi, svn:author, svn:date and svn:log are revision properties - changes to these don't appear in the log.
>>>>
>>>> Cheers,
>>>> Daniel
>>>> Sent from my phone.
>>>>
>>>
>>> I started working on this and I think I completed the changes for
>>> svn_ra_local. Attached is the patch. This is a work in progress. I would
>>> like to get some initial comments/suggestions on the patch and would
>>> like to know whether I am proceeding on right direction.
>>>
>>> This patch adds new option '--ignore-properties' to 'log' sub
>>> command. If this option is provided then command ignore revisions that
>>> has only property changes from output.
>>
>> I haven't reviewed the patch, but would like to discuss the approach.
>>
>> Ignoring all or none of a set of prop mods may not be granular enough.
>> Since consumers rarely change mergeinfo without changing content, I
>> don't see that the new behavior would be exercised very often. What
>> we really want to do is filter out mergeinfo changes on a per-node
>> basis, not a per-revision basis as the new docs claim. (Perhaps the
>> docs are inaccurate?)
>>
>> I don't see anywhere in the patch where you address RA layers other
>> than ra_local. Also, what is the expected behavior when operating
>
> I mentioned this in the mail. I will be handling this in the future. I
> just wanted get feedback from list so that I might not end up direction
> less.
>
>> against older servers? If you send the ignore_properties flag to
>> older servers, they will just ignore the flag and return all the
>> revisions, and the client will dutifully report them. Do you have a
>> plan for this scenario?
>
> Thank you for pointing this out. So far I have not thought about
> this. What about throwing an error, if there the server version is less
> than what we expect?
Or more specifically, if the server doesn't support the required
capability (as on the ignore-mergeinfo-log branch).
>> Finally, is there a reason why you chose to do this against trunk,
>> rather than the ignore-mergeinfo-log branch, which already addresses
>> many of the above concerns? I've been hacking on that branch a bit
>> lately, and it isn't complete but patches and review are definitely
>> welcome.
>>
>
> I did this against trunk because I could see lots of changes in
> ignore-mergeinfo-log branch. For me to avoid confusion, I thought I will
> start preparing patch against trunk. Also if I include this option also
> to the ignore-mergeinfo-log branch, will that be possible to factor out
> my changes alone? Or is it a good idea to have a separate branch for
> this. I am not sure at this point.
>
> Do you mean to say that ignore-mergeinfo-log has an option to filter out
> revisions being send to the receiver if that revision contains only
> property changes? Isn't that issue 3690 all about?
Well, reading back through issue 3690, it looks like it could use some
design work. :)
The reporter states the request, and then this motivation: "many users
are not interested in reviewing changes to property changes and only
care about content changes." To that I would ask "why?" What
scenarios does this pop up in.
I know my specific usecase is managing the release branches, and
having to filter through all the svn:mergeinfo mods when attempting to
find the last time CHANGES was textually modified. Maybe that would
be accomplished by a 'ignore all prop mods' flag; maybe not. Maybe
the approach on the ignore-mergeinfo-log branch is *too* discrete, and
would end up being a performance killer; maybe not. The more I write,
the the more I think this deserves some design consideration. (And
hence my desire to do it on a branch rather than trunk: I don't want
to hold up 1.7, and don't want to release a half-cooked idea.)
One other option may be to have some way of using the ignored prop mod
list, as on the branch, to specify ignoring *all* prop mods,
irrespective of property name. From an API perspective, this could
accomplish both goals. But I still think we need to think more about
what problem we're trying to solve.
Incidentally, I like the issue number: 3690. For bonus points, can
anybody tell my why that number is significant in Subversion-land? :)
-Hyrum
PS - Sorry if I was a bit harsh in the last mail.
Received on 2011-02-01 06:08:13 CET