Code review comment for lp:~gz/juju-core/openstack_network_config_1241674

Revision history for this message
William Reade (fwereade) wrote :

quick comments -- thoughts?

https://codereview.appspot.com/52710048/diff/1/provider/openstack/config.go
File provider/openstack/config.go (right):

https://codereview.appspot.com/52710048/diff/1/provider/openstack/config.go#newcode42
provider/openstack/config.go:42: "network": "",
this feels like maybe it should be an Omit?

https://codereview.appspot.com/52710048/diff/1/provider/openstack/config.go#newcode95
provider/openstack/config.go:95: return c.attrs["network"].(string)
and this a `,ok`?

https://codereview.appspot.com/52710048/diff/1/provider/openstack/local_test.go
File provider/openstack/local_test.go (right):

https://codereview.appspot.com/52710048/diff/1/provider/openstack/local_test.go#newcode364
provider/openstack/local_test.go:364: "network":
"f81d4fae-7dec-11d0-a765-00a0c91e6bf6",
a thought: default-network? private-network?

https://codereview.appspot.com/52710048/diff/1/provider/openstack/local_test.go#newcode371
provider/openstack/local_test.go:371: c.Assert(err, gc.ErrorMatches,
"(?s)cannot run instance: .*itemNotFound.*")
I'd like to see a more precise error check here really.

https://codereview.appspot.com/52710048/

« Back to merge proposal