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