Gabriela Gibson wrote on Thu, Dec 27, 2012 at 22:09:40 +0000: > On 27/12/12 17:24, Daniel Shahaf wrote:>> /* Acquire a lock (of sorts) > on the repository associated with the > >> * given RA SESSION. This lock is just a revprop change attempt in a > >> - * time-delay loop. This function is duplicated by svnrdump in > >> - * load_editor.c. > >> + * time-delay loop. A similar function used by svnrdump is defined in > >> + * svnrdump/load_editor.c > > > > Why did you s/duplicate/similar/? I think the former is better (for its > > "if you change me, change that other one too" implication). > > > > That's all. All in all I'm not sure whether to just apply it or wait > > for another iteration, I'll go for the latter. Thanks for the patch! > > > > Daniel > > > Hi Daniel, > > Second iteration attached. I've copied the indentation style from one > of your commit messages. >
Wrong attachment? It seems you re-attached the first iteration. > With regard to the get_lock() functions, my reading was that the two > functions now had different arguments and different behaviour - to my > mind, in that case, "duplicate" isn't an appropriate description. > True. But to me, "duplicate" has a subtext of "if you update one of them, update the other too", that's why I prefer it. (Also, the functions really are duplicates: the svnrdump one is identical to the svnsync one with a curried steal_lock=FALSE.) > Of course, the fact that I think this comment is improved by rewording > doesn't make it so - feel free to make the final call on this. Well, between two opinions, I'll fall back to "Status quo wins" --- at least until there's a third opinion.
