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: Corrected incorrect instances of (setup|set up) #2692

Merged
merged 3 commits into from
Sep 28, 2020
Merged

Docs: Corrected incorrect instances of (setup|set up) #2692

merged 3 commits into from
Sep 28, 2020

Conversation

philnichol
Copy link
Contributor

@philnichol philnichol commented Sep 28, 2020

What this PR does / why we need it:
Corrects incorrect usage of (setup|set up) in the docs
Which issue(s) this PR fixes:
No linked issue

Special notes for your reviewer:
I ran this in Grafana and they suggested I do it here too.
Checklist

  • Documentation added
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented Sep 28, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@oddlittlebird oddlittlebird left a comment

Choose a reason for hiding this comment

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

Thanks! Couple of editing suggestions.

@@ -88,7 +88,7 @@ value is larger than the hash of the stream. When the replication factor is
larger than 1, the next subsequent tokens (clockwise in the ring) that belong to
different ingesters will also be included in the result.

The effect of this hash set up is that each token that an ingester owns is
The effect of this hash setup is that each token that an ingester owns is
Copy link
Contributor

Choose a reason for hiding this comment

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

This version of "setup" is a noun. Please revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously happy to revert this, but shouldn't it be a noun?
Or should it be "The effect of this hash being set up is that..."?

Copy link
Contributor

Choose a reason for hiding this comment

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

"Setup" is the noun version of the word. If you revert it and change nothing else, then it is correct.

Your version is also correct, but would require more words. I suggest going with the reversion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotcha, apologies! Cheers for the feedback, I've reverted that change

Copy link
Contributor

Choose a reason for hiding this comment

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

If you are ready for another review, I suggest clicking "Re-request review." People don't always notice comments, but the request for review sends a separate (and more visible) notification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, didn't know that!

docs/sources/clients/aws/eks/_index.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2020

Codecov Report

Merging #2692 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2692      +/-   ##
==========================================
+ Coverage   61.48%   61.51%   +0.02%     
==========================================
  Files         173      173              
  Lines       13412    13412              
==========================================
+ Hits         8246     8250       +4     
+ Misses       4411     4408       -3     
+ Partials      755      754       -1     
Impacted Files Coverage Δ
pkg/logql/evaluator.go 92.38% <0.00%> (-0.43%) ⬇️
pkg/querier/queryrange/downstreamer.go 97.64% <0.00%> (+2.35%) ⬆️
pkg/promtail/targets/file/tailer.go 73.03% <0.00%> (+4.49%) ⬆️

Copy link
Contributor

@oddlittlebird oddlittlebird left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing to our docs!

@owen-d owen-d merged commit 851dbc2 into grafana:master Sep 28, 2020
@owen-d
Copy link
Member

owen-d commented Sep 28, 2020

Thanks!

@philnichol philnichol deleted the docs-correcting-setup branch September 28, 2020 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants