Reviewers: mp+218152_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: refactor --series to use StringsValue

seriesVar is now called seriesValue and is implemented using
StringsValue. This reuses StringValue while preserving the series
validation logic.

https://code.launchpad.net/~menno.smits/juju-core/1302005-bootstrap_series/+merge/218152

(do not edit description out of merge proposal)


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

Affected files (+28, -22 lines):
   A [revision details]
   M cmd/juju/bootstrap.go
   M cmd/juju/upgradejuju.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-20140501192736-36p3425ohtpy4np7
+New revision: <email address hidden>

Index: cmd/juju/bootstrap.go
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2014-04-30 22:05:31 +0000
+++ cmd/juju/bootstrap.go	2014-05-02 19:25:05 +0000
@@ -6,7 +6,6 @@
  import (
  	"fmt"
  	"os"
-	"strings"

  	"launchpad.net/gnuflag"

@@ -81,7 +80,7 @@
  	c.EnvCommandBase.SetFlags(f)
  	f.Var(constraints.ConstraintsValue{Target:  
&c.Constraints}, "constraints", "set environment constraints")
  	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.Var(newSeriesValue(nil, &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")
  }
@@ -106,6 +105,30 @@
  	return cmd.CheckEmpty(args)
  }

+type seriesValue struct {
+	*cmd.StringsValue
+}
+
+// newSeriesValue is used to create the type passed into the  
gnuflag.FlagSet Var function.
+func newSeriesValue(defaultValue []string, target *[]string) *seriesValue {
+	v := seriesValue{(*cmd.StringsValue)(target)}
+	*(v.StringsValue) = defaultValue
+	return &v
+}
+
+// Implements gnuflag.Value Set.
+func (v *seriesValue) Set(s string) error {
+	if err := v.StringsValue.Set(s); err != nil {
+		return err
+	}
+	for _, name := range *(v.StringsValue) {
+		if !charm.IsValidSeries(name) {
+			return fmt.Errorf("invalid series name %q", name)
+		}
+	}
+	return nil
+}
+
  // Run connects to the environment specified on the command line and  
bootstraps
  // a juju in that environment if none already exists. If there is as yet  
no environments.yaml file,
  // the user is informed how to create one.
@@ -174,22 +197,3 @@
  		Placement:   c.Placement,
  	})
  }
-
-type seriesVar struct {
-	target *[]string
-}
-
-func (v seriesVar) Set(value string) error {
-	names := strings.Split(value, ",")
-	for _, name := range names {
-		if !charm.IsValidSeries(name) {
-			return fmt.Errorf("invalid series name %q", name)
-		}
-	}
-	*v.target = names
-	return nil
-}
-
-func (v seriesVar) String() string {
-	return strings.Join(*v.target, ",")
-}


Index: cmd/juju/upgradejuju.go
=== modified file 'cmd/juju/upgradejuju.go'
--- cmd/juju/upgradejuju.go	2014-04-07 10:05:04 +0000
+++ cmd/juju/upgradejuju.go	2014-05-02 19:25:05 +0000
@@ -79,7 +79,7 @@
  	c.EnvCommandBase.SetFlags(f)
  	f.StringVar(&c.vers, "version", "", "upgrade to specific version")
  	f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of  
tools")
-	f.Var(seriesVar{&c.Series}, "series", "upload tools for supplied  
comma-separated series list")
+	f.Var(newSeriesValue(nil, &c.Series), "series", "upload tools for  
supplied comma-separated series list")
  }

  func (c *UpgradeJujuCommand) Init(args []string) error {