Re: Some improvements to cvs2svn (patches)

Michael Haggerty <[email protected]> Thu, 24 Mar 2011 06:13:57 +0100
Newsgroups gmane.comp.version-control.subversion.cvs2svn.devel
Message-ID <[email protected]>
Robin,

Thanks for the patches!

On 03/23/2011 05:37 PM, Robin wrote:
> I needed some improvements in cvs2svn and committed them to my fork on github:
> 
> https://github.com/robinst/cvs2svn/commits/master
> 
> There are three unrelated changes:
> 
> • Add missing import for FatalError

Good catch.  Committed as trunk r5329.

> • Add ConditionalPropertySetter

Looks good.  Committed as trunk r5330.

I suggest that we write a few predicates for obvious things, like
cvs_file_is_binary or even cvs_file_has_mode('b').  This would make it
convenient for users to use without having to know what a lambda is or
write a Python function.  Then it would make much sense to wrap the
"KeywordHandlingPropertySetter('collapsed')" lines in the DVCS option
files to make them only apply to non-binary files, which would make them
a safer default than the present version.

If you are interested, go for it.

> • Add option for excluding paths from conversion

I noticed that the patch uses os.path.relpath().  But this function was
only added in Python 2.6, whereas we are still trying to remain
compatible with Python 2.4.  So please find an alternative.  You might
consider using common.normalize_svn_path() to normalize the excluded
paths and compare that to something like
common.path_join(cvs_directory.get_cvs_path(), fname) (though make sure
that Attic directories are handled the way you want).  The advantage of
using Subversion paths is that it doesn't matter whether the user uses
'/' or '\' as path separator on Windows.  Otherwise it is probably best
to put the paths through os.path.normpath() somewhere to make sure that
all slashes lean the same way.

It would also be nice if this patch included changes to the docstrings
of functions taking new parameters (including especially whether the
paths are filesystem vs. Subversion formatted and what path they are
relative to).  There should also be examples and explanatory comments in
the cvs2*-example.options files, too; otherwise nobody will ever find
these options.

Other than those points, I think that this feature is a good idea and
would be happy to add it.

Thanks again!
Michael

-- 
Michael Haggerty
[email protected]
http://softwareswirl.blogspot.com/

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

To unsubscribe from this discussion, e-mail: [[email protected]].