Cli multiple clusters #859
Conversation
| func runClusterDefault(args *docopt.Args) error { | ||
| name := args.String["<cluster-name>"] | ||
|
|
||
| if config.SetDefault(name) { |
There was a problem hiding this comment.
Flip the conditional, so that we avoid indenting code:
if !config.SetDefault(name) {
return nil
}
if err := config.SaveTo(configPath()); err != nil {
return err
}
...
|
Nice work! I'd say we need a few integration tests (see |
| @@ -39,7 +40,7 @@ func (c *Config) Marshal() []byte { | |||
| return buf.Bytes() | |||
| } | |||
|
|
|||
| func (c *Config) Add(s *Cluster) error { | |||
| func (c *Config) Add(s *Cluster, force bool, set_as_default bool) error { | |||
There was a problem hiding this comment.
default or setDefault (Golang uses camel case)
|
@archseer This seems weird. The recent build reports that |
|
The test failure is a bug in the TOML decoding library, the |
| if err := config.Add(s); err != nil { | ||
| f := args.Bool["--force"] | ||
| d := args.Bool["--default"] | ||
| if err := config.Add(s, f, d); err != nil { |
There was a problem hiding this comment.
Instead of passing the default flag into config.Add, I'd call config.SetDefault right after calling config.Add:
if err := config.Add(s, args.Bool["--force"]); err != nil {
return err
}
if args.Bool["--default"] {
if err := config.SetDefault(s.Name); err != nil {
return err
}
}
|
Yes, the TOML bug is fixed on the latest version, so wait until #957 is merged, then rebase on master. |
|
Also, it'd be good to know which cluster is the default, so |
|
Finally green! 😄 |
|
@geovanisouza92 Great, can you squash these up into a single commit? :) /cc @flynn/devs for a review |
| remove removes a cluster from the ~/.flynnrc configuration file | ||
| add adds a cluster to the ~/.flynnrc configuration file | ||
| remove removes a cluster from the ~/.flynnrc configuration file | ||
| default set the default cluster |
There was a problem hiding this comment.
set or print the default cluster
|
This looks good! It can be merged after my minor comments are addressed and rebased on top of master. |
|
@titanous RFC ✨ |
|
|
||
| // Remove existing match | ||
| c.Clusters = append(c.Clusters[:i], c.Clusters[i+1:]...) | ||
| msg = "" |
There was a problem hiding this comment.
No need to do this anymore.
|
@geovanisouza92 One last change, then we merge! :) |
cli: Adding "force" flag to override cluster config cli: Adding cluster "default" flag and command cli: Remove cluster from config when --force cli: Use default cluster from ~/.flynnrc cli: Renaming parameters cli: Test global cluster flag cli: Test force flag to replace config cli: Test change default cluster cli: Testing default cluster config cli: Remove if-else-if cli: Add default cluster emits message cli: Swap lines to avoid toml decoding issue cli: Simplify set default cluster cli: Show default cluster in list cli: Set first cluster as default cli: Enhance message when cluster does not exist cli: Scoping message on Config.Add() Signed-off-by: Geovani de Souza <geovanisouza92@gmail.com>
|
Code changed, rebased on master, but fails on Anyway, considering that this is my 2nd Go code, I think that's awesome, thanks for you patience and guidance! 😄 |
|
LGTM, thanks! (I restarted the CI build) |
Solves #190