On Mon, Jan 31, 2011 at 10:52 PM, Noorul Islam K M <noo...@collab.net> wrote: > Hyrum K Wright <hy...@hyrumwright.org> writes: > >> 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 > > 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.
