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

eshost --add: support inclusion of --tags (see details for examples) #31

Merged
merged 1 commit into from
Aug 2, 2017

Conversation

rwaldron
Copy link
Collaborator

@rwaldron rwaldron commented Aug 1, 2017

  • eshost --add ch ch which ch --tags latest
  • eshost --add ch ch which ch --tags latest,greatest

Tests will follow in another PR which contains all the tests that I've written

Signed-off-by: Rick Waldron waldron.rick@gmail.com

@rwaldron rwaldron requested a review from bterlson August 1, 2017 18:16
Copy link
Collaborator

@dilijev dilijev left a comment

Choose a reason for hiding this comment

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

LGTM

@rwaldron rwaldron force-pushed the add-tags-with-host branch from 1d4a09d to 61d171b Compare August 1, 2017 18:39
path = Path.join(process.cwd(), path);
}

config.hosts[name] = { type, path, args };
if (tags) {
if (typeof tags === 'string') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Meh. I suppose these could be combined into a single expression

config.hosts[name] = { type, path, args };
if (tags) {
if (typeof tags === 'string') {
tags = [tags];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this already taken care of with hostTags = argv.tags.split(','); (and then hostTags is passed in for the tags param to this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed it is. I will correct this.

- eshost --add ch ch `which ch` --tags latest
- eshost --add ch ch `which ch` --tags latest,greatest

Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
@rwaldron rwaldron force-pushed the add-tags-with-host branch from 61d171b to 97c9cda Compare August 2, 2017 17:37
@rwaldron
Copy link
Collaborator Author

rwaldron commented Aug 2, 2017

Assuming the "approval" is still valid, I'd like to include this in tested features, so I'm landing it.

@rwaldron rwaldron merged commit c1acdd3 into bterlson:master Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants