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

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Please take a look.

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

https://codereview.appspot.com/30190043/diff/20001/cloudinit/sshinit/configure_test.go#newcode136
cloudinit/sshinit/configure_test.go:136: assertScriptMatches(c, cfg,
"(.|\n)*apt-get -y update(.|\n)*", false)
On 2013/11/22 06:37:50, wallyworld wrote:
> To prevent typos causing the test to pass, I'd love to see the regexp
string
> which is cut and pasted be put into a const or var.

Done.

https://codereview.appspot.com/30190043/diff/20001/cloudinit/sshinit/configure_test.go#newcode149
cloudinit/sshinit/configure_test.go:149: assertScriptMatches(c, cfg,
"(.|\n)*apt-get -y upgrade(.|\n)*", false)
On 2013/11/22 06:37:50, wallyworld wrote:
> ditto

Done.

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

https://codereview.appspot.com/30190043/diff/20001/provider/common/bootstrap.go#newcode37
provider/common/bootstrap.go:37: // TODO(axw) 2013-11-22 #XXX
On 2013/11/22 06:37:50, wallyworld wrote:
> a bug number would be great here

Done.

https://codereview.appspot.com/30190043/diff/20001/provider/common/bootstrap.go#newcode90
provider/common/bootstrap.go:90: fmt.Fprintln(ctx.Stderr, "Stopping
instance...")
On 2013/11/22 06:37:50, wallyworld wrote:
> pedant alert - I think we should use case consistently for out
messages.
> "Stopping...." vs "cleaning up..." etc

Absolutely - just an oversight, thanks for picking it up.

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

« Back to merge proposal