[PATCH] convert: Perforce source for conversion to Mercurial

Frank A. Kingswood frank at kingswood-consulting.co.uk
Tue Mar 3 14:55:43 CST 2009


Mads Kiilerich wrote:
> Here comes a nit-picking review. All opinions are my own and in no way 

Thanks for the review comments, in which you picked up one serious 
problem. A new patch follows, here are some answers

> These meta-comments are probably not relevant in the version that ends 
> up getting applied.

I've now discovered "hg email --intro".

>> +        for f in loaditer(stdout):
>>   
> 
> What is "f"? Perhaps a more descriptive name would be more self-documenting?

These are the meaningless dictionaries that p4 uses to deliver its 
output. As throwaway dictionary objects I'd rather keep their names 
short. They are called d now, to match the generator function.

>> +            while "depotFile%d" % i in f and "rev%d" % i:
> IMHO this looks suspicious. It might be correct and clever, but the 
> intention isn't obvious to me.
BUG

>> +        if mode is None:
>> +            raise IOError()
> 
> Is it really an IOError?

Apparently so. The other sources do this too.

Frank



More information about the Mercurial-devel mailing list