Re: Where do CVS "descriptions" get stored?

Michael Haggerty <[email protected]>
Newsgroups gmane.comp.version-control.subversion.cvs2svn.user
Message-ID <[email protected]>
Tim Landscheidt wrote:
> Michael Haggerty <[email protected]> wrote some time ago:
>> [...]
>> If you submit a clean patch I would be happy to include it in the
>> cvs2svn source tree.
> 
> I finally got around to take a look at it (despite your in-
> structions for dummies :-)); it works for me. Things to con-
> sider: [...]

Hi,

Thanks a lot for the patch.  It looks good and I will definitely commit
something like it.  It is very nice to avoid losing this information
when converting from CVS.

There is no need to be modest about your lack of Python experience; now
you are in the club!

I have a few comments.

1. Normally we like submitters to suggest a comment to include with the
commit.  This is not crucial, but it is your chance to explain what you did.

2. We would definitely require a test case for this feature to make sure
that it doesn't break due to some later changes.  I put a simple one
together quickly using the Cvs2SvnPropertiesTestCase and will attach the
patch for your information/comments.  If it is OK with you, I can commit
it myself after I commit your patch.

3. Regarding the choice of property name: I agree that we shouldn't add
something to the "svn:*" namespace (at least not without clearing it
with the Subversion project first).  But "dc:" seems rather obscure.
The only property that we invent so far is called "cvs2svn:cvs-rev", and
is set on a file to record the CVS revision number.  (This is not a
recommended feature but it is still supported.)  So cvs2svn:description
would be one possibility.  But I don't see why the description (or
cvs-rev!) has anything to do with cvs2svn; probably more appropriate
names would be cvs:description and cvs:rev-num.  Would you be OK with
"cvs:description"?

Also, it would be easy to make the property name changeable by the user;
just add a constructor parameter to your property setter, store it in a
member variable, and use that when setting the property:

class DescriptionPropertySetter(SVNPropertySetter):
    # ...
    def __init__(self, propname='cvs:description'):
        self.propname = propname

And change the examples accordingly.  If you don't want to add this, I
will probably add it myself after committing your patch.

4. I noticed that you make this the default behavior for cvs2svn,
overrideable only by using an options file.  That is fine with me.  But
if anybody else has any objections, now is the time to air them.

5. Let's mention this in the CHANGES file, since it is a nice new
feature.  You can add a change to the CHANGES file in your patch;
otherwise, I'll add it myself.

Thanks!
Michael

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1670&dsMessageId=2415569

To unsubscribe from this discussion, e-mail: [[email protected]].
cvs-description-test.diff (text/x-diff, 1 KB)
diff --git a/run-tests.py b/run-tests.py
index 04805b3..0e4f864 100755
--- a/run-tests.py
+++ b/run-tests.py
@@ -3697,6 +3697,16 @@ def many_deletes():
     ))
 
 
+cvs_description = Cvs2SvnPropertiesTestCase(
+    'main',
+    doc='test handling of CVS file descriptions',
+    props_to_test=['dc:description'],
+    expected_props=[
+        ('trunk/proj/default', ['This is an example file description.']),
+        ('trunk/proj/sub1/default', [None]),
+        ])
+
+
 ########################################################################
 # Run the tests
 
@@ -3897,6 +3907,7 @@ test_list = [
     unlabeled_branch_name_collision,
     collision_with_unlabeled_branch_name,
     many_deletes,
+    cvs_description,
     ]
 
 if __name__ == '__main__':
diff --git a/test-data/main-cvsrepos/proj/default,v b/test-data/main-cvsrepos/proj/default,v
index 3b68382..2a98627 100644
--- a/test-data/main-cvsrepos/proj/default,v
+++ b/test-data/main-cvsrepos/proj/default,v
@@ -44,7 +44,7 @@ next	;
 
 
 desc
-@@
+@This is an example file description.@
 
 
 1.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.