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

Re: [PATCH] Re: Regarding issue 3690 - Work in progress

From: Hyrum K Wright <hyrum_at_hyrumwright.org>
Date: Mon, 31 Jan 2011 13:44:14 -0600

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

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.

Anyway, these are my thoughts, I'm interested in what others have to say.

-Hyrum
Received on 2011-01-31 20:44:53 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.