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

Fixing typo in readme for varnish plugin #4188

Closed
wants to merge 3 commits into from

Conversation

anthosz
Copy link

@anthosz anthosz commented May 23, 2018

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson
Copy link
Contributor

Thanks for the fix, could you change it to use snake_case instance_name though? Either one should work but we usually use this style in the config.

@danielnelson danielnelson added this to the 1.6.4 milestone May 23, 2018
@danielnelson danielnelson added the fix pr to fix corresponding bug label May 23, 2018
@anthosz
Copy link
Author

anthosz commented May 24, 2018

@danielnelson This is now done.
Btw, all variables has been move to 'snake_case'
There are a procedure to inform the users of changes after updates or just add a point in Changelog? (with this last commit, current telegraf.conf will not works with future version of this plugin)

@danielnelson
Copy link
Contributor

danielnelson commented May 24, 2018

Sorry, I wasn't clear enough about what needed switched. We only want to use snake_case in the sample config file, but we use the normal Go style camelCase in the code.

The toml parser will deserialize instance_name into InstanceName automatically

@danielnelson
Copy link
Contributor

@anthosz I was updated the docs based on this pull request in 449bd5c

@anthosz anthosz deleted the patch-1 branch June 5, 2018 06:15
@anthosz
Copy link
Author

anthosz commented Jun 5, 2018

@danielnelson Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants