[PATCH 1 of 3] convert: cvsps.py - code to generate changesets from a CVS repository

[Frank Kingswood]

Matt Mackall wrote:
[many review comments]

Thanks for your review comments.
> We prefer not to use names_with_underbars or CapitalizedNames. Even for
> classes or members.
>   
Hmm, al right then.
>> +
>> +    # reusing strings typically saves about 40% of memory
>>     
> Very interesting.
>   
This is peculiar to what cvsps is trying to do. Changeset log messages 
will be identical and there might be hundreds of copies in a large 
changeset. This is even more obvious in the pickle that cvsps stores.
> Try/except turns out to be fairly slow. It's faster and simpler to do:
>
> if s not in _scache:
>     _scache[s] = s
> return _scache[s]
>
> But in this case, we can get by with:
>
> return _scache.setdefault(s, s)
>   
Good one, thanks. I was just following GvR's assertion that it should be 
fast enough.
>> +        cachefile = ['-'.join(re.findall(r'\w+', s)) for s in cachefile if s]
>> +        cachefile = os.path.join(cachedir, '.'.join(cachefile))
>>     
>
> No idea what's happening there.
>   
The cvsps cache pickle needs a uniquified name, based on the repository 
location. The address may have all sort of nasties in it, slashes, 
colons and such. So here I take just the alphanumerics, concatenated in 
a way that does not mix up the various components. For example, one 
could have two repositories
    :pserver:user at server:/path
and
    /pserver/user/server/path
and they need to be mapped to different cache file names.
I'll put a comment in the code along these lines.
> This all looks pretty good and I'm inclined to take it, but I'd still
> like to see some naming and whitespace fixups. In particular, you've
> still got lots of things like a = x+y. 
>   
But that *is* clean :-)

Frank