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.

Sign up for GitHub

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

Jump to bottom

Doc: Vague link text. #2698

Merged
merged 9 commits into from
Oct 12, 2020
Merged

Doc: Vague link text. #2698

merged 9 commits into from
Oct 12, 2020

Conversation

bemasher
Copy link
Contributor

What this PR does / why we need it:
Fixes vague link text in the documentation.

Which issue(s) this PR fixes:
Fixes #2680

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented Sep 30, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@bemasher
Copy link
Contributor Author

For anyone that wants to review the links further down the road, I wrote a quick tool for parsing the documentation and listing them.

https://gist.github.com/bemasher/0f1387c593683237cf943800a8ecce44

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2020 •
edited
Loading

Codecov Report

Merging #2698 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2698   +/-   ##
=======================================
  Coverage   61.40%   61.40%           
=======================================
  Files         173      173           
  Lines       13446    13446           
=======================================
  Hits         8257     8257           
- Misses       4431     4437    +6     
+ Partials      758      752    -6
Impacted Files Coverage Δ
pkg/promtail/positions/positions.go 46.80% <0.00%> (-11.71%) ⬇️
pkg/logql/evaluator.go 92.81% <0.00%> (+0.42%) ⬆️
pkg/promtail/targets/file/filetarget.go 66.19% <0.00%> (+2.11%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.64% <0.00%> (+2.35%) ⬆️
pkg/promtail/targets/file/tailer.go 73.33% <0.00%> (+4.44%) ⬆️

oddlittlebird
oddlittlebird suggested changes Sep 30, 2020
View reviewed changes
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.

Wow, much improved! I have a few suggestions to make it even better.

docs/sources/alerting/_index.md Outdated Show resolved Hide resolved
docs/sources/clients/aws/ec2/_index.md Outdated Show resolved Hide resolved
docs/sources/alerting/_index.md Outdated Show resolved Hide resolved
docs/sources/clients/aws/ec2/_index.md Outdated Show resolved Hide resolved
docs/sources/getting-started/labels.md Outdated Show resolved Hide resolved
@owen-d
Copy link
Member

owen-d commented Sep 30, 2020

Once @oddlittlebird's suggestions are implemented, looks good to me.

Thanks!

@bemasher bemasher requested a review from oddlittlebird October 1, 2020 03:22
achatterjee-grafana
achatterjee-grafana reviewed Oct 1, 2020
View reviewed changes
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Made a few copy-edit suggestions.

@bemasher bemasher changed the title Docs: Vague link text. Doc: Vague link text. Oct 4, 2020
@bemasher
Copy link
Contributor Author

bemasher commented Oct 9, 2020

Github doesn't seem to think I've addressed @oddlittlebird's requested changes. What do I need to do to fix this?

@owen-d
Copy link
Member

owen-d commented Oct 9, 2020

Ok, this is looking great. I left a few smalll nitpicks and there are some upstream conflicts that need resolving then this is ready to be merged 🎉 .

@oddlittlebird
Copy link
Contributor

I cleared my block. Please address @owen-d's comments and clear the merge conflicts.

bemasher and others added 4 commits October 10, 2020 22:52
@bemasher
Fixes remove vague link text #2680
beb9f58
@bemasher
Fix remaining 'docs' links.
856e6db
@bemasher @oddlittlebird
Accept suggestion.
51092eb 
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
@bemasher @oddlittlebird
Change HTTP API to Ruler API.
1254b42 
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
bemasher and others added 5 commits October 10, 2020 22:53
@bemasher @oddlittlebird
Correct typo.
83f0741 
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
@bemasher @oddlittlebird
Capitalize Github.
e3c8ffa 
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
@bemasher @oddlittlebird
Capitalize Promtail.
30e17f3 
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
@bemasher
Add copy-edit suggestions from @achatterjee-grafana.
5ebf0f9
@bemasher
Incorporate @owen-d's changes.
b0db3ef
@bemasher
Copy link
Contributor Author

Alright, I think I've covered everything.

@owen-d owen-d merged commit e644095 into grafana:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@achatterjee-grafana achatterjee-grafana achatterjee-grafana left review comments

@owen-d owen-d owen-d left review comments

@oddlittlebird oddlittlebird oddlittlebird approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove vague link text
6 participants
@bemasher @CLAassistant @codecov-commenter @owen-d @oddlittlebird @achatterjee-grafana