-
Notifications
You must be signed in to change notification settings - Fork 309
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
Clarify README docs for template_name and templates #1043
Conversation
Clarify when the index template is created and what 'hash' means (this was not at all obvious to me as a newcomber to Ruby - not all fluentd users will know Ruby)
README.md
Outdated
@@ -541,7 +541,7 @@ The path to the file containing the template to install. | |||
|
|||
### templates | |||
|
|||
Specify index templates in form of hash. Can contain multiple templates. | |||
Specify index templates (to be created on startup) in the form of a Ruby hash (accepts JSON dict). Can contain multiple templates. |
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.
Hash should be obvious for HashMap for normal Rubyists. We should revert this part.
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.
I came to this documentation as a user of fluentd (and this plugin) without having any Ruby experience - personally I wouldn't expect someone using (not developing) something written in language X to necessarily know about language X, what do you think?
And even once I eventually realised that 'hash' needed interpreting in the context of ruby it still wasn't obvious to me that a JSON dict string was the appropriate form to provide.
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.
Even if newbie for Ruby and Fluentd, I didn't accept your proposed change. Because hash should be used in everywhere for Ruby world. So, I would like to insist revert that part.
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.
Ok I will remove the word "Ruby" and leave it just as "hash" - I think the other changes on this line (clarifying when it is created, and what format it is in) are valuable so before I push more commits, are you happy with this?
Specify index templates (to be created on startup) in the form of a hash (accepts JSON dict). Can contain multiple templates.
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.
Yes. I love to use your suggestion. 😉
@@ -527,7 +527,7 @@ into same document with data1 as wanted and duplicated document is avoided. | |||
|
|||
### template_name | |||
|
|||
The name of the template to define. If a template by the name given is already present, it will be left unchanged, unless [template_overwrite](#template_overwrite) is set, in which case the template will be updated. | |||
The name of the index template to create on fluentd startup. If a template by the name given is already present, it will be left unchanged, unless [template_overwrite](#template_overwrite) is set, in which case the template will be updated. |
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.
👍
README.md
Outdated
@@ -541,7 +541,7 @@ The path to the file containing the template to install. | |||
|
|||
### templates | |||
|
|||
Specify index templates in form of hash. Can contain multiple templates. | |||
Specify index templates (to be created on startup) in the form of a Ruby hash (accepts JSON dict). Can contain multiple templates. |
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.
Yes. I love to use your suggestion. 😉
Hopefully this is now ok @cosmo0920 |
Noted. Thank you! |
Clarify when the index template is created and what 'hash' means (this was not at all obvious to me as a newcomber to Ruby - not all fluentd users will know Ruby)
I expect none of the below are really relevant as it is a readme-only change?
(check all that apply)
version
in gemspec are untouchedelasticsearch_dynamic
(not required but recommended)