Hi, Daniel Shahaf writes: > 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: > > > 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.
Ok, let's have this copy for now. > > > 2. If you do duplicate code, then add big comments (in all instances of > > > the duplication) pointing to the other instances. > > > > +1 I've mentioned it in the commit message, but alright- I'll add a comment in the file. > > > 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.) Ok, let me know when it's done. I'll port it to svnrdump too. -- Ram
