Skip to content
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

Merged
merged 33 commits into from
Nov 4, 2021

Conversation

tylerslaton
Copy link
Contributor

@tylerslaton tylerslaton commented Oct 21, 2021

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

  • Initial Cobra structure
  • Eval subcommand
  • Generator and Combination packages
  • Initial Kube Controller code generation
  • Unit testing for all packages

Closes #1

Tyler Slaton and others added 22 commits September 29, 2021 15:15
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>
@tylerslaton tylerslaton changed the title Initial combo cli Adding Combo CLI Binary Oct 21, 2021
Copy link
Member

@awgreene awgreene left a 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.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines 21 to 23
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.
Copy link
Member

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=

Copy link
Contributor Author

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?

internal/cmd/eval.go Outdated Show resolved Hide resolved
pkg/combination/combination.go Outdated Show resolved Hide resolved
Comment on lines 113 to 126
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)
}
}
}
Copy link
Member

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.

pkg/combination/combination_test.go Outdated Show resolved Hide resolved
pkg/combination/combination_test.go Outdated Show resolved Hide resolved
pkg/generator/generator_test.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tylerslaton
Copy link
Contributor Author

#8 Has an update to the README as discussed earlier.

@awgreene
Copy link
Member

awgreene commented Nov 4, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2021
@tylerslaton tylerslaton merged commit 46847ad into operator-framework:main Nov 4, 2021
@tylerslaton tylerslaton deleted the initial-combo-cli branch November 4, 2021 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

combo eval command
3 participants