Code review comment for lp:~axwalk/juju-core/synchronous-bootstrap

Please take a look.

https://codereview.appspot.com/30190043/diff/60001/cloudinit/sshinit/configure_test.go
File cloudinit/sshinit/configure_test.go (right):

https://codereview.appspot.com/30190043/diff/60001/cloudinit/sshinit/configure_test.go#newcode36
cloudinit/sshinit/configure_test.go:36:
environs.RegisterProvider("sshinit", &testProvider{})
On 2013/12/02 14:54:09, fwereade wrote:
> Can we stick an explicit "test" into the name somewhere please?

Done.

https://codereview.appspot.com/30190043/diff/60001/environs/bootstrap/state_test.go
File environs/bootstrap/state_test.go (right):

https://codereview.appspot.com/30190043/diff/60001/environs/bootstrap/state_test.go#newcode59
environs/bootstrap/state_test.go:59: c.Assert(err, gc.IsNil) // doesn't
exist, juju don't carea
On 2013/12/02 14:54:09, fwereade wrote:
> s/carea/care/

Done.

https://codereview.appspot.com/30190043/diff/60001/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/30190043/diff/60001/provider/common/bootstrap.go#newcode40
provider/common/bootstrap.go:40: // Std{in,out,err}, and interrupt
signal handling.
On 2013/12/02 14:54:09, fwereade wrote:
> I'd like this to be an imminent followup, please :).

No problems.

https://codereview.appspot.com/30190043/diff/60001/provider/common/bootstrap.go#newcode61
provider/common/bootstrap.go:61: inst, hw, err = env.StartInstance(cons,
selectedTools, machineConfig)
On 2013/12/02 14:54:09, fwereade wrote:
> Wouldn't this still work with :=, even given inst? Maybe I'm still
sluggish
> after lunch...

No, you're right, that'll work. Fixed.

https://codereview.appspot.com/30190043/diff/60001/provider/common/testing/bootstrap.go
File provider/common/testing/bootstrap.go (right):

https://codereview.appspot.com/30190043/diff/60001/provider/common/testing/bootstrap.go#newcode17
provider/common/testing/bootstrap.go:17: f :=
func(*common.BootstrapContext, instance.Instance,
*cloudinit.MachineConfig) error {
On 2013/12/02 14:54:09, fwereade wrote:
> Let's log something in here so that there's some way of diagnosing
what happened
> if Disable gets called in a surprising situation.

Done.

https://codereview.appspot.com/30190043/diff/60001/provider/maas/maas_test.go
File provider/maas/maas_test.go (right):

https://codereview.appspot.com/30190043/diff/60001/provider/maas/maas_test.go#newcode14
provider/maas/maas_test.go:14: commontesting
"launchpad.net/juju-core/provider/common/testing"
On 2013/12/02 14:54:09, fwereade wrote:
> Oh, a thought. Might DisableFinishBootstrap be happier in
environs/testing?

Hmm yeah that's fine, and stops the package proliferation. Moved.

https://codereview.appspot.com/30190043/diff/60001/utils/ssh/ssh.go
File utils/ssh/ssh.go (left):

https://codereview.appspot.com/30190043/diff/60001/utils/ssh/ssh.go#oldcode15
utils/ssh/ssh.go:15: // Move this to a common package for use in
cmd/juju, and others.
On 2013/12/02 14:54:09, fwereade wrote:
> Can the SSHy bits in cmd/juju use this now?

Done. Also updated environs/sshstorage, and environs/manual. Had to
tweak utils/ssh a bit, and add an ScpCommand function.

https://codereview.appspot.com/30190043/