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

[cli/dev] rely on commander for deduping argv #14181

Merged
merged 1 commit into from
Sep 26, 2017

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Sep 26, 2017

In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.

In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.
@spalger spalger added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc review v6.0.0 v6.1.0 v7.0.0 labels Sep 26, 2017
@spalger spalger requested a review from w33ble September 26, 2017 19:21
Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That did it, including --plugin-path multiple times works as expected again. Thanks for taking care of this!

@w33ble
Copy link
Contributor

w33ble commented Sep 26, 2017

@spalger it looks like this change was made way back in 5.2, which means it's been broken for a while.

It would be great if you could backport this fix, at least to the next 5.6 release, but 5.5 would be even nicer.

@spalger spalger merged commit 014dee0 into elastic:master Sep 26, 2017
spalger added a commit that referenced this pull request Sep 26, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.

(cherry picked from commit 014dee0)
spalger added a commit that referenced this pull request Sep 26, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.

(cherry picked from commit 014dee0)
spalger added a commit that referenced this pull request Sep 26, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.

(cherry picked from commit 014dee0)
spalger added a commit that referenced this pull request Sep 26, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.

(cherry picked from commit 014dee0)
@spalger
Copy link
Contributor Author

spalger commented Sep 26, 2017

5.5: 80def10
5.6: 3ef06eb
6.0: 69686c4
6.x/6.1: 9713688

@spalger spalger deleted the fix/dev-cli-multi-flags branch September 26, 2017 20:40
@epixa epixa removed the v6.0.0-rc2 label Sep 28, 2017
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 20, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Dec 1, 2017
In eb31780 I started combining custom argv with the process's argv, and naively used union to combine the lists. This breaks cli arguments that are supposed to be repeated and isn't necessary since commander handles parsing the argv and deduping/merging based on config.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v5.6.3 v6.0.0 v6.1.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants