[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: Ramkumar Ramachandra <artagnon_at_gmail.com>
Date: Mon, 20 Sep 2010 10:35:34 +0530

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
Received on 2010-09-20 07:07:45 CEST

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