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 mdox remote/local link checking and move web preprocess #4357

Merged
merged 5 commits into from
Aug 17, 2021

Conversation

saswatamcode
Copy link
Member

This PR adds mdox local and remote link checking with smart validation.

cc: @bwplotka

Note: This fixes all links in docs. So while all links will work from GitHub now, website links may break. This will be fixed in subsequent PRs where current preprocess will be replaced by mdox tranform.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice, I am afraid we might want to add transform in one go...

.mdox.validate.yaml Outdated Show resolved Hide resolved
docs/components/sidecar.md Show resolved Hide resolved
@saswatamcode saswatamcode changed the title docs: Add mdox remote and local link checking docs: Add mdox remote/local link checking and move web preprocess Jun 23, 2021
@saswatamcode saswatamcode force-pushed the mdox-links branch 2 times, most recently from b14d745 to 5bcdee5 Compare June 24, 2021 08:01
bwplotka
bwplotka previously approved these changes Jun 24, 2021
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

This looks amazing, LGTM mod minor nits. 💪🏽


validators:
# Smart validators to skip checking PR/issue links of Thanos, Prometheus and Cortex.
- regex: 'thanos-io\/thanos'
Copy link
Member

Choose a reason for hiding this comment

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

Regex github.com -> github

Copy link
Member

Choose a reason for hiding this comment

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

Let's make it smarter

# 301 errors even when curl-ed.
- regex: 'envoyproxy\.io'
type: 'ignore'
# Due to rate limiting, mdox link checking is a time consuming process.
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove that

.mdox.yaml Outdated
Comment on lines 64 to 76
- glob: "components/README.md"
path: /components/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"

- glob: "operating/README.md"
path: /operating/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"

- glob: "contributing/README.md"
path: /contributing/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if just one element will be enough:

Suggested change
- glob: "components/README.md"
path: /components/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"
- glob: "operating/README.md"
path: /operating/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"
- glob: "contributing/README.md"
path: /contributing/_index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"
- glob: "**/README.md"
path: _index.md
frontMatter:
template: |
title: "{{ .Origin.FirstHeader }}"

.mdox.yaml Show resolved Hide resolved
Makefile Show resolved Hide resolved
@bill3tt
Copy link
Contributor

bill3tt commented Jul 6, 2021

@saswatamcode I think we're ready to merge 🚀 just need to fix the CI docs check. You will be able to repro this failure locally by running make check-docs.

@saswatamcode
Copy link
Member Author

Hi @ianbillett! This PR has few blockers such as this and merging right now will break the website for previous version docs. I'll be raising PRs soon to fix those in mdox and in previous Thanos release branches and update this PR as well! This can be merged then! 🤗

@saswatamcode saswatamcode changed the title docs: Add mdox remote/local link checking and move web preprocess [WIP] docs: Add mdox remote/local link checking and move web preprocess Aug 1, 2021
@bwplotka bwplotka changed the title [WIP] docs: Add mdox remote/local link checking and move web preprocess docs: Add mdox remote/local link checking and move web preprocess Aug 3, 2021
bwplotka
bwplotka previously approved these changes Aug 3, 2021
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice, amazing! LGTM, just probably we need to update release branches. Let's do a script for this job, run and then merge this. WDYT?

.mdox.yaml Outdated
lastmod: "{{ .Origin.LastMod }}"
backMatter:
template: |
Found a typo, inconsistency or missing information in our docs?
Copy link
Member

Choose a reason for hiding this comment

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

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks, amazing!

@bwplotka bwplotka merged commit 9d6b004 into thanos-io:main Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants