Merge lp://qastaging/~thumper/juju-core/supercommand-missing-func into lp://qastaging/~juju/juju-core/trunk

Reviewers: mp+164824_code.launchpad.net,

Message:
Please take a look.

Description:
Callbacks in supercommand for missing subcommands

In order to support plugins in a generic way, we want the supercommand
to be
able to call out to another function if a subcommand was asked for that
wasn't
one of the registered commands.

https://code.launchpad.net/~thumper/juju-core/supercommand-missing-func/+merge/164824

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/9609043/

Affected files:
   A [revision details]
   M cmd/supercommand.go
   M cmd/supercommand_test.go

LGTM with a suggestion.

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go#newcode8
cmd/supercommand_test.go:8:
why the extra blank line?

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go#newcode123
cmd/supercommand_test.go:123: var (
i thought you said you have very low tolerance to duplicated code :) can
you unify these 2 (or possibly all 3) similar setup blocks in a helper
and call it in all 3 test cases?

https://codereview.appspot.com/9609043/

Please take a look.

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go#newcode8
cmd/supercommand_test.go:8:
On 2013/05/21 07:46:57, dimitern wrote:
> why the extra blank line?

One thing I have started doing is to put he standard library imports
before the local project imports. I feel that this gives a better
separation.

https://codereview.appspot.com/9609043/diff/1/cmd/supercommand_test.go#newcode123
cmd/supercommand_test.go:123: var (
On 2013/05/21 07:46:57, dimitern wrote:
> i thought you said you have very low tolerance to duplicated code :)
can you
> unify these 2 (or possibly all 3) similar setup blocks in a helper and
call it
> in all 3 test cases?

I added a helper method, and also refactored the tests so there isn't so
much duplication, and we are tighter in what each test is testing.

https://codereview.appspot.com/9609043/

LGTM with a minor typo to be fixed

https://codereview.appspot.com/9609043/diff/5001/cmd/supercommand.go
File cmd/supercommand.go (right):

https://codereview.appspot.com/9609043/diff/5001/cmd/supercommand.go#newcode229
cmd/supercommand.go:229: // never called.
Should the comment say Info and not Init?

https://codereview.appspot.com/9609043/

*** Submitted:

Callbacks in supercommand for missing subcommands

In order to support plugins in a generic way, we want the supercommand
to be
able to call out to another function if a subcommand was asked for that
wasn't
one of the registered commands.

R=dimitern, wallyworld
CC=
https://codereview.appspot.com/9609043

https://codereview.appspot.com/9609043/diff/5001/cmd/supercommand.go
File cmd/supercommand.go (right):

https://codereview.appspot.com/9609043/diff/5001/cmd/supercommand.go#newcode229
cmd/supercommand.go:229: // never called.
On 2013/05/21 22:24:20, wallyworld wrote:
> Should the comment say Info and not Init?

Yes it should.

https://codereview.appspot.com/9609043/