Please take a look.
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()) On 2013/10/15 07:08:34, axw wrote: > What if the user has specified -s, -r, and -e on the command line? Is this > really an error then?
Hmmm. I thought yes when I wrote this. But I changed it so that it works more as expected.
> Just a thought: perhaps ImageMetadataCommand could implement HasRegion. If > environ doesn't implement HasRegion, fall back to ImageMetadataCommand.
See my changes - I think it's ok as is.
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" On 2013/10/15 07:08:34, axw wrote: > Move me please
Done.
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") On 2013/10/15 07:08:34, axw wrote: > You can just use s.PatchEnvironment here, which wraps testbase.PatchEnvironment > & AddCleanup.
https://codereview.appspot.com/14502059/
« Back to merge proposal
Please take a look.
https:/ /codereview. appspot. com/14502059/ diff/1/ cmd/plugins/ juju-metadata/ imagemetadata. go juju-metadata/ imagemetadata. go (right):
File cmd/plugins/
https:/ /codereview. appspot. com/14502059/ diff/1/ cmd/plugins/ juju-metadata/ imagemetadata. go#newcode72 juju-metadata/ imagemetadata. go:72: return "environment %q cannot provide region and endpoint",
cmd/plugins/
fmt.Errorf(
environ.Name())
On 2013/10/15 07:08:34, axw wrote:
> What if the user has specified -s, -r, and -e on the command line? Is
this
> really an error then?
Hmmm. I thought yes when I wrote this. But I changed it so that it works
more as expected.
> Just a thought: perhaps ImageMetadataCo mmand could implement mmand.
HasRegion. If
> environ doesn't implement HasRegion, fall back to
ImageMetadataCo
See my changes - I think it's ok as is.
https:/ /codereview. appspot. com/14502059/ diff/1/ cmd/plugins/ juju-metadata/ imagemetadata_ test.go juju-metadata/ imagemetadata_ test.go (right):
File cmd/plugins/
https:/ /codereview. appspot. com/14502059/ diff/1/ cmd/plugins/ juju-metadata/ imagemetadata_ test.go# newcode20 juju-metadata/ imagemetadata_ test.go: 20: "path/filepath"
cmd/plugins/
On 2013/10/15 07:08:34, axw wrote:
> Move me please
Done.
https:/ /codereview. appspot. com/14502059/ diff/1/ cmd/plugins/ juju-metadata/ imagemetadata_ test.go# newcode42 juju-metadata/ imagemetadata_ test.go: 42: restore := PatchEnvironmen t("AWS_ ACCESS_ KEY_ID" , "access") PatchEnvironmen t
cmd/plugins/
testbase.
On 2013/10/15 07:08:34, axw wrote:
> You can just use s.PatchEnvironment here, which wraps
testbase.
> & AddCleanup.
Done.
https:/ /codereview. appspot. com/14502059/