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

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

Please take a look.

https://codereview.appspot.com/91740043/diff/1/cmd/juju/bootstrap.go
File cmd/juju/bootstrap.go (right):

https://codereview.appspot.com/91740043/diff/1/cmd/juju/bootstrap.go#newcode107
cmd/juju/bootstrap.go:107: if err != instance.ErrPlacementScopeMissing {
On 2014/04/24 03:10:01, wallyworld wrote:
> What happens if there's a different error other than
ErrPlacementScopeMissing?
> In that case we should just return that other error shouldn't we? ie
there's a
> difference between an unsupported placement directive and a placement
directive
> that doesn't parse due to some other error. Even if there's no other
error that
> can be returned (yet), reworking the conditional would make it
explicit that we
> are only interested in a specific case.

There are other errors, but they're not meaningful to us if we don't
support scoped placements. For example, lxc:non-numeric will fail to
parse, but we bootstrap doesn't care about why; it only cares that
"lxc:anything" is not supported by bootstrap.

https://codereview.appspot.com/91740043/

« Back to merge proposal