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