Hi Daniel, Daniel Shahaf writes: > Ramkumar Ramachandra wrote on Sun, Sep 19, 2010 at 16:52:50 +0530: > > 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: > > > > > 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. > > > > Please add a comment to svnsync's get_lock() as well.
+1 needed :) [[[ * subversion/svnsync/main.c (get_lock): Add a comment explaining what the function does along with a note about it being duplicated in svnrdump. * subversion/svnrdump/load_editor.c (get_lock): Add a comment explaining what the function does along with a note about it being duplicated in svnsync. ]]] Index: subversion/svnsync/main.c =================================================================== --- subversion/svnsync/main.c (revision 998652) +++ subversion/svnsync/main.c (working copy) @@ -285,7 +285,13 @@ check_lib_versions(void) /* Acquire a lock (of sorts) on the repository associated with the - * given RA SESSION. + * 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. + * + * ## TODO: Make this function more generic and + * expose it through a header for use by other Subversion + * applications to avoid duplication. */ static svn_error_t * get_lock(svn_ra_session_t *session, apr_pool_t *pool) Index: subversion/svnrdump/load_editor.c =================================================================== --- subversion/svnrdump/load_editor.c (revision 998772) +++ subversion/svnrdump/load_editor.c (working copy) @@ -56,6 +56,14 @@ commit_callback(const svn_commit_info_t *commit_in return SVN_NO_ERROR; } +/* 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 svnsync in main.c. + * + * ## TODO: Make this function more generic and + * expose it through a header for use by other Subversion + * applications to avoid duplication. + */ static svn_error_t * get_lock(svn_ra_session_t *session, apr_pool_t *pool) { > Thanks for the offer. I can do that myself, though; svnrdump is > a first-class citizen now, as is this duplication (per the direction > this thread seems to be going), so AFAICT all cards say the branch > maintainer should be adding that to svnrdump. Sure :) -- Ram