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.
Received on 2012-12-28 00:45:40 CET