Code review comment for lp:~wallyworld/juju-core/upgrade-remove-old-config

Revision history for this message
Tim Penhey (thumper) wrote :

LGTM - although not a fan of all the value asserts in the setup.

https://codereview.appspot.com/70230049/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/70230049/diff/1/cmd/jujud/machine.go#newcode121
cmd/jujud/machine.go:121: logger.Infof("machine agent %v start (%s)",
a.Tag(), version.Current)
I'm sure I saw this somewhere else, missing prereq?

https://codereview.appspot.com/70230049/diff/1/upgrades/deprecatedattributes_test.go
File upgrades/deprecatedattributes_test.go (right):

https://codereview.appspot.com/70230049/diff/1/upgrades/deprecatedattributes_test.go#newcode46
upgrades/deprecatedattributes_test.go:46:
c.Assert(allAttrs["public-bucket"], gc.Equals, "foo")
Seems a lot to me to have the asserts in setup for this

https://codereview.appspot.com/70230049/

« Back to merge proposal