Code review comment for lp:~axwalk/juju-core/lp1313960-bootstrap-to

Reviewers: mp+217837_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: bootstrap --to <placement>

Change bootstrap to use "--to <placement>",
to make way for repurposing positional args.

Fixes lp:1313960

https://code.launchpad.net/~axwalk/juju-core/lp1313960-bootstrap-to/+merge/217837

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/90970043/

Affected files (+15, -19 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140430030329-cl8i2buuaypidtg4
+New revision: <email address hidden>

Index: cmd/juju/bootstrap.go
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2014-04-24 12:33:19 +0000
+++ cmd/juju/bootstrap.go 2014-04-30 22:05:31 +0000
@@ -72,7 +72,6 @@
  func (c *BootstrapCommand) Info() *cmd.Info {
   return &cmd.Info{
    Name: "bootstrap",
- Args: "[placement]",
    Purpose: "start up an environment from scratch",
    Doc: bootstrapDoc,
   }
@@ -84,6 +83,7 @@
   f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of
tools before bootstrapping")
   f.Var(seriesVar{&c.Series}, "series", "upload tools for supplied
comma-separated series list")
   f.StringVar(&c.MetadataSource, "metadata-source", "", "local path to use
as tools and/or metadata source")
+ f.StringVar(&c.Placement, "to", "", "a placement directive indicating an
instance to bootstrap")
  }

  func (c *BootstrapCommand) Init(args []string) (err error) {
@@ -96,20 +96,14 @@
   }
   // Parse the placement directive. Bootstrap currently only
   // supports provider-specific placement directives.
- placement, err := cmd.ZeroOrOneArgs(args)
- if err != nil {
- return err
- }
- if placement == "" {
- return nil
- }
- _, err = instance.ParsePlacement(placement)
- if err != instance.ErrPlacementScopeMissing {
- // We only support unscoped placement directives for bootstrap.
- return fmt.Errorf("unsupported bootstrap placement directive %q",
placement)
- }
- c.Placement = placement
- return nil
+ if c.Placement != "" {
+ _, err = instance.ParsePlacement(c.Placement)
+ if err != instance.ErrPlacementScopeMissing {
+ // We only support unscoped placement directives for bootstrap.
+ return fmt.Errorf("unsupported bootstrap placement directive %q",
c.Placement)
+ }
+ }
+ return cmd.CheckEmpty(args)
  }

  // Run connects to the environment specified on the command line and
bootstraps

Index: cmd/juju/bootstrap_test.go
=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go 2014-04-25 04:23:08 +0000
+++ cmd/juju/bootstrap_test.go 2014-04-30 22:05:31 +0000
@@ -376,12 +376,12 @@
   },
  }, {
   info: "placement",
- args: []string{"something"},
+ args: []string{"--to", "something"},
   placement: "something",
  }, {
- info: "invalid placement: ssh",
- args: []string{"ssh:someplace"},
- err: `unsupported bootstrap placement directive "ssh:someplace"`,
+ info: "additional args",
+ args: []string{"anything", "else"},
+ err: `unrecognized args: \["anything" "else"\]`,
  }}

  func (s *BootstrapSuite) TestBootstrapTwice(c *gc.C) {