Code review comment for lp:~thumper/juju-core/juju-run-cli

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run.go
File cmd/juju/run.go (right):

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run.go#newcode33
cmd/juju/run.go:33: Run the commands on the specified targets.
This just occurred to me: should we be supporting patterns, like in juju
status?
e.g. juju run --service='nova-*' <commands>

Just a thought.

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run.go#newcode128
cmd/juju/run.go:128: var results = make([]interface{}, 0)
Why are you initialising this with 0 length, and then appending? Why not
initialise with len(runResults) and assign elements?

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run.go#newcode139
cmd/juju/run.go:139: values["Stdout"] = string(result.Stdout)
You're assuming that stdout/stderr are UTF-8 here, which they may not
be. At the least, can we please use utf8.Valid before converting them to
strings?

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run.go#newcode203
cmd/juju/run.go:203: type RunClient interface {
Nice

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run_test.go
File cmd/juju/run_test.go (right):

https://codereview.appspot.com/50310043/diff/1/cmd/juju/run_test.go#newcode210
cmd/juju/run_test.go:210: },
In light of my comments in run.go, I think a test for binary
stdout/stderr is in order.

https://codereview.appspot.com/50310043/