On Mon, Jan 31, 2011 at 10:53 AM, Noorul Islam K M <noo...@collab.net> wrote: > Daniel Becroft <djcbecr...@gmail.com> writes: > >> On 27/01/2011, at 17:04, Noorul Islam K M <noo...@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
