Code review comment for lp:~mathepic/bzr/mkdir-recursive

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Thanks for the patch. Some minor things to fix ...

1. This is a boolean option so the parameter ought to be declared as "parent=False" instead of "parent=None".

2. It should also be tested as simply "if parents" instead of testing against None.

3. You'll need to tweak the help argument in the Option declaration so that it starts with a capital letter and ends with a full stop. Otherwise, it won't merge because our style checker will fall over!

BTW, "bzr init" has a "create-prefix" option that does a very similar thing. I think consistency with posix mkdir (as you've done) makes more sense. Maybe we ought to change init down the track to be consistent with this? If so, it's outside the scope of this patch. I was just mentioning it for completeness.

review: Needs Fixing

« Back to merge proposal