Code review comment for lp:~jspashett/bzr/587868_args_handling_cant_debug

Revision history for this message
Alexander Belchenko (bialix) wrote :

2 more fixes needed:

1) you can't raise InternalBzrError without importing this name first, so please add this line before raise:

    from bzrlib.errors import InternalBzrError

and sorry for my typo in error class name, it's actually InternalBzrError not BzrInternalError.

2) Bazaar code guideline requires only 2 blank lines between high-level functions, so please remove extra blank line after `return argv`

And just a note: I'm usually trying to create minimal possible patch and avoid changing names of variables unless it's strictly necessary, because it's just creates a noise which makes review harder. Your change of args to argv is not strictly necessary here, although I don't mind with such change. It won't block your patch. But such change don't add any real value to the patch.

« Back to merge proposal