Code review comment for lp:~axwalk/juju-core/envcmd-inversion

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM with perhaps a 2nd opinion from William.

It does seem that we are missing tests to ensure that each command is
wrapped as expected. We test the wrapping function itself in
environmentcommand but I can't see how or where we are testing that each
business command is wrapped eg for add-machine, how do we test that it
has been registered using
jujucmd.Register(wrap(&AddMachineCommand{}))and not just
jujucmd.Register(&AddMachineCommand{})

We should be able to iterate over all registered commands to check. Can
we do this before landing?

https://codereview.appspot.com/94350045/

« Back to merge proposal