Code review comment for lp:~axwalk/juju-core/juju-failed-bootstrap-destroy-jenv

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

Please take a look.

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

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode118
cmd/juju/bootstrap.go:118:
On 2014/02/03 08:32:22, rog wrote:
> I'd prefer it if result had "err" in its name here,
> either by actually naming it "err", or perhaps by
> naming it "resultErr".

Done.

https://codereview.appspot.com/59560043/diff/1/cmd/juju/bootstrap.go#newcode135
cmd/juju/bootstrap.go:135:
On 2014/02/03 08:32:22, rog wrote:
> I think this case is unusual enough that we don't actually need to
special
> "config-only" flag. The error message will probably contain the file
name
> anyway. I think a better approach would be for destroy-environment to
delete the
> config file if it gets a definitive "not found" error when trying to
delete the
> environment.

Okay. I'm going to take the --config-only bit out of the CL for now.
That can be addressed in a followup.

https://codereview.appspot.com/59560043/

« Back to merge proposal