[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Update to out-of-date code comments

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Fri, 28 Dec 2012 01:44:41 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.