-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding Combo CLI Binary #4
Adding Combo CLI Binary #4
Conversation
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
Signed-off-by: Tyler Slaton <tyslaton@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Tayler, I left some feedback.
internal/cmd/eval.go
Outdated
Use: "eval", | ||
Short: "Evaluate the combinations for a file at the given path", | ||
Long: `Evaluate the combinations for a file at the given path. The file provided must be valid YAML. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a part of this commit, but the description for the completion
command is not capitalized:
$ bin/combo help
Create combinations of kubernetes manifests
Usage:
combo [command]
Available Commands:
completion generate the autocompletion script for the specified shell
eval Evaluate the combinations for a file at the given path
help Help about any command
Flags:
-h, --help help for combo
Use "combo [command] --help" for more information about a command.bin/combo help=
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to remove this command as it is auto-included in Cobra?
pkg/combination/combination.go
Outdated
var helper func(combo map[string]string, i int) | ||
helper = func(combo map[string]string, i int) { | ||
for _, val := range arrays[i] { | ||
combo[replacements[i]] = val | ||
if i == max { | ||
// Append a copy of the map to the combos | ||
comboCopy := map[string]string{} | ||
copier.Copy(&comboCopy, &combo) | ||
combos = append(combos, comboCopy) | ||
} else { | ||
helper(combo, i+1) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be it's own function.
cc5a247
to
3f3a582
Compare
6874eaa
to
710dcdc
Compare
710dcdc
to
14b36e3
Compare
a46b3b7
to
96000da
Compare
… outlined in helm
#8 Has an update to the README as discussed earlier. |
/lgtm |
Summary
Supersedes PR #3
Adding the initial structure, logic, and general basis for the Combo CLI tool as defined by this repo's README.
Additions
Closes #1