Review: Approve
Looks good.
[0]
I agree with Francesco's suggestions. Well, all suggestions but one: I think
you should keep "if len(words) > 0:" because it's more specific than using "if
words". For the same reason, if you want to test if "something" is None, we
would rather write explicitly "if something is not None:" rather than "if
something:".
[1]
147 + self.assertEqual(
148 + {'one': True,
149 + 'two': False,
150 + },
151 + SampleCommandCollection.parsingParameters())
This is really a detail but I think you could write this as
self.assertEqual(
{'one': True, 'two': False},
SampleCommandCollection.parsingParameters())
It would (artificially) reduce the SLOC and I think it's more readable. But
again, it's really a detail so feel to change it or not.
--
https://code.launchpad.net/~ivo-kracht/launchpad/bug-425934/+merge/110786
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
