-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Multiple task cli option #143
Conversation
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.
thx. just a coule of minor things
@@ -198,7 +200,10 @@ func runAdHoc(ctx context.Context, targets []string, r *runner.Process) error { | |||
|
|||
// runGen generates a destination report for the task's targets | |||
func runGen(opts options, r *runner.Process) (err error) { | |||
targets := targetsForTask(opts.Targets, opts.TaskName, r.Playbook) | |||
var targets []string | |||
for _, taskName := range opts.TaskNames { |
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.
probably this will cause duplicated targets in the final list. I'm not sure how bad the end result is going to be but deduplication of the result slice sounds like the right thing to do
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.
Actually, it causes duplicates in the targets list. I've updated the code and added a test to cover the case.
cc21f8c
to
c862a41
Compare
LGTM, can you pls rebase/squash into a single commit, and I'll merge |
c5a744c
to
a691c33
Compare
All changes are squashed and rebased on the latest master. |
Support for multiple
--task
CLI option requested in #131