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

Re: svn commit: r998502 - /subversion/trunk/subversion/svnrdump/load_editor.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 19 Sep 2010 11:03:43 +0100

Stefan Sperling wrote on Sun, Sep 19, 2010 at 11:40:49 +0200:
> On Sun, Sep 19, 2010 at 10:29:58AM +0100, Daniel Shahaf wrote:
> > artagnon_at_apache.org wrote on Sat, Sep 18, 2010 at 17:54:50 -0000:
> > > +static svn_error_t *
> > > +get_lock(svn_ra_session_t *session, apr_pool_t *pool)
> > > +{
...
> > > +}
> >
> > 1. Please don't duplicate code.
>
> I think it's fine for svnrdump to have its own copy of this for now.
> We could at some point merge them into libsvn_subr/ of course, but I
> don't see a huge problem with just 2 instances.
>
> > 2. If you do duplicate code, then add big comments (in all instances of
> > the duplication) pointing to the other instances.
>
> +1
>
> > 3. Incidentally, I have modified the svnsync instance of this function
> > on the atomic-revprops branch. So the desire to avoid duplication isn't
> > just theoretical in this case...
>
> We should fix the race in svnrdump on the atomic-revprop branch as well.
>

No problem; r998622. (The svnsync patch isn't committed yet, but it
should be easy to port it to svnrdump.)

> Stefan
Received on 2010-09-19 12:05:01 CEST

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