Code review comment for lp://staging/~dave-cheney/juju-core/071-cmd-jujud-stater-I

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go
File cmd/jujud/main.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/main.go#newcode22
cmd/jujud/main.go:22: jujud.Register(&StaterCommand{})
Either this needs a *stater.Config (-1), or the command itself needs to
set one up itself (+1); ISTM that it doesn't, and the tests poke all
that info in directly. Did I miss something?

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go
File cmd/jujud/stater.go (right):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode12
cmd/jujud/stater.go:12: *stater.Config
Why embed a (nil) pointer when you could just embed directly...

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode22
cmd/jujud/stater.go:22: }
...initialize the fields here...

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater.go#newcode27
cmd/jujud/stater.go:27: sr := stater.NewStater(s.Config)
...and pass &s.Config here?

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go
File cmd/jujud/stater_test.go (left):

https://codereview.appspot.com/7149043/diff/4001/cmd/jujud/stater_test.go#oldcode16
cmd/jujud/stater_test.go:16: ctx := &cmd.Context{
There are a lot of these scattered through the codebase, but maybe it's
not worth factoring them into one place. Thoughts?

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (left):

https://codereview.appspot.com/7149043/diff/4001/environs/cloudinit/cloudinit.go#oldcode135
environs/cloudinit/cloudinit.go:135: if err := addMongoToBoot(c, cfg);
err != nil {
I don't think this change is justified yet -- we don't pass any of the
necessary information in. (Does this actually run correctly? I don't see
how it can...)

https://codereview.appspot.com/7149043/