Code review comment for lp:~wallyworld/juju-core/improve-image-metadata-command

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

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imagemetadata.go
File cmd/plugins/juju-metadata/imagemetadata.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imagemetadata.go#newcode72
cmd/plugins/juju-metadata/imagemetadata.go:72: return
fmt.Errorf("environment %q cannot provide region and endpoint",
environ.Name())
What if the user has specified -s, -r, and -e on the command line? Is
this
really an error then?

Just a thought: perhaps ImageMetadataCommand could implement HasRegion.
If environ doesn't implement HasRegion, fall back to
ImageMetadataCommand.

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imagemetadata_test.go
File cmd/plugins/juju-metadata/imagemetadata_test.go (right):

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode20
cmd/plugins/juju-metadata/imagemetadata_test.go:20: "path/filepath"
Move me please

https://codereview.appspot.com/14502059/diff/1/cmd/plugins/juju-metadata/imagemetadata_test.go#newcode42
cmd/plugins/juju-metadata/imagemetadata_test.go:42: restore :=
testbase.PatchEnvironment("AWS_ACCESS_KEY_ID", "access")
You can just use s.PatchEnvironment here, which wraps
testbase.PatchEnvironment & AddCleanup.

https://codereview.appspot.com/14502059/

« Back to merge proposal