I'm still a little unclear as to the desired behavior outcome and its
testing with this branch, as it currently tested it doesn't seem
particularly useful.
The use of the mixin, also feels like its adding complexity here, around
subclass ordering and proper lineage to customize_parser vs. render
(which have inverse call lineages). Incorporating into the base class
might be cleaner via class flag switch.
I'm still a little unclear as to the desired behavior outcome and its
testing with this branch, as it currently tested it doesn't seem
particularly useful.
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ commands. py commands. py (right):
File juju/hooks/
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ commands. py#newcode18 commands. py:18: class TestOptionMixin (object) :
juju/hooks/
missing doc string describing purpose/usage.
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ commands. py#newcode34 commands. py:34: self.handle_ test(result) handle_ test behavior.
juju/hooks/
there's a lot of unesc. method delegation here. just inline the
test_true/
The use of the mixin, also feels like its adding complexity here, around
subclass ordering and proper lineage to customize_parser vs. render
(which have inverse call lineages). Incorporating into the base class
might be cleaner via class flag switch.
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ commands. py#newcode114 commands. py:114: def render(self, result):
juju/hooks/
Why is this returning None, why is it implemented at all? ditto for the
two other implementations of the same here.
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ tests/test_ invoker. py tests/test_ invoker. py (right):
File juju/hooks/
https:/ /codereview. appspot. com/6216053/ diff/6001/ juju/hooks/ tests/test_ invoker. py#newcode356 tests/test_ invoker. py:356:
juju/hooks/
this is testing something very different then what the cli usage would
commonly be, namely the cli usage would be testing for values unset.
For values set, a null value seems valid, has the go team documented the
behavior of this cli param?
https:/ /codereview. appspot. com/6216053/