OCM-3801 | Feat | Added default values for CIDRs and host prefix #552
Conversation
cmd/ocm/create/cluster/cmd.go
Outdated
| @@ -595,6 +623,11 @@ func preRun(cmd *cobra.Command, argv []string) error { | |||
| return err | |||
| } | |||
|
|
|||
| if args.interactive { | |||
| machineCIDR, serviceCIDR, podCIDR, hostPrefix := GetDefaultClusterFlavors(connection, args.flavour) | |||
There was a problem hiding this comment.
is the order of variables/matching the return values? serviceCIDR and podCIDR seem inverted
pkg/arguments/interactive.go
Outdated
| // If the default value is nil, set the value as a default value | ||
| if flag.DefValue == "<nil>" { | ||
| flag.DefValue = flag.Value.String() | ||
| } |
There was a problem hiding this comment.
It's not clear to me why we need to set the flag default value here, can you explain, please?
There was a problem hiding this comment.
We set the default value so that it is set (and shown) on the console when the user does not input any values. When we set the value in https://github.com/openshift-online/ocm-cli/pull/552/files#diff-2f19822c49dc0bec88a73920ea0f04a30b7047b5a827c90447e02b7378744f97R627, it sets the "value" attribute and not the defValue attribute.
There was a problem hiding this comment.
This is the implementation of the // TODO below, right? You can probably move that comment here - it will be more clear.
| prompt := &survey.Input{ | ||
| Message: getQuestion(flag), | ||
| Help: flag.Usage, | ||
| // TODO respect flag default, if set | ||
| // (awkward because https://github.com/golang/go/issues/39516). |
There was a problem hiding this comment.
I would keep the comment with the link to this issue which explains the nil issue.
Changed
Tested