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

docs: Add example output section to HTTP plugin #11497

Merged
merged 3 commits into from
Jul 14, 2022
Merged

docs: Add example output section to HTTP plugin #11497

merged 3 commits into from
Jul 14, 2022

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Jul 12, 2022

This pull request:

  • Adds a example output section to the http plugin readme to resolve linter error I noticed here: https://github.com/influxdata/telegraf/runs/7306583417?check_suite_focus=true, this failing in an unrelated PR is probably something that needs to be fixed in the github action but I didn't address this.
  • Make the linter more lenient and doesn't mark links against the long line rule, it looks like the logic is simpler now using the actual string to get the length as well. Still seems accurate when testing it locally.

@telegraf-tiger telegraf-tiger bot added the docs Issues related to Telegraf documentation and configuration descriptions label Jul 12, 2022
@sspaink sspaink changed the title docs: Add example output section to HTTP plugin README chore: Update readme linter to ignore links when checking line length Jul 12, 2022
@sspaink sspaink added chore and removed docs Issues related to Telegraf documentation and configuration descriptions labels Jul 12, 2022
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Hmm ignoring the URL text means that reading these in a text editor doesn't actually make the line length 80 characters anymore. And the user will have random lines that shoot out.

@sspaink
Copy link
Contributor Author

sspaink commented Jul 13, 2022

True, but I was thinking maybe links could be the exception? In this case the link even on its own line exceed the 80 character limit. My main motivation is just to get the green check mark, so I could perhaps just shorten the URL (this seems legit?) or not include it? Although the example output seems a bit random without the context of the article.

@powersj
Copy link
Contributor

powersj commented Jul 13, 2022

My main motivation is just to get the green check mark, so I could perhaps just shorten the URL (this seems legit?) or not include it?

Have you seen the reference-style link method? I wasn't aware of this till Dave went through updating the other READMEs
https://www.markdownguide.org/basic-syntax/#reference-style-links

This example output was taken from [this instructional article][1].

[1]: https://docs.influxdata.com/telegraf/v1.21/guides/using_http/

@sspaink sspaink changed the title chore: Update readme linter to ignore links when checking line length docs: Add example output section to HTTP plugin Jul 14, 2022
@sspaink sspaink added docs Issues related to Telegraf documentation and configuration descriptions and removed chore labels Jul 14, 2022
@sspaink
Copy link
Contributor Author

sspaink commented Jul 14, 2022

I didn't know you could do that! Seems like a good solution to me, I've updated the PR accordingly.

@telegraf-tiger
Copy link
Contributor

@sspaink sspaink merged commit 2d357d4 into master Jul 14, 2022
@sspaink sspaink deleted the updatehttpdoc branch July 14, 2022 14:38
MyaLongmire pushed a commit that referenced this pull request Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues related to Telegraf documentation and configuration descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants