[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