-
Notifications
You must be signed in to change notification settings - Fork 297
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
Link fixes #737
Link fixes #737
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #737 +/- ##
==========================================
- Coverage 77.56% 77.41% -0.16%
==========================================
Files 176 176
Lines 5308 5299 -9
==========================================
- Hits 4117 4102 -15
- Misses 1191 1197 +6
|
44696c2
to
f89813c
Compare
uses: lycheeverse/[email protected] | ||
with: | ||
fail: true | ||
args: "--threads 1 --max-concurrency 1 --verbose --no-progress './**/*.md' './**/*.html'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this repository there is tons of links to the GitHub. It leads to throttling. One of the way is to use once request at a time.
uses: lycheeverse/[email protected] | ||
with: | ||
fail: true | ||
args: "--threads 1 --max-concurrency 1 --verbose --no-progress './**/*.md' './**/*.html'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider removing './**/*.html'
and reverting the workflow name to markdownlint
(if the name change would lead to any problem).
For me it could stay as it is 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File filers are taken from default lychee configuration. There is no possibility to set other custom attributes and not setting file filters.
I would keep as it is.
uses: actions/[email protected] | ||
|
||
- name: verify links by lycheeverse | ||
uses: lycheeverse/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you know if this is used widely? Any idea how opentelemetry.io folks deal with this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used in:
- https://github.com/open-telemetry/opentelemetry-go/blob/ad45631b53faa74191fcee37c8f010e520af67e1/.github/workflows/links.yml#L19
- https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/blob/b0d4457610231e5e3a8af32f8856a9d0b0fbdf8d/.github/workflows/validate-documentation.yml#L28
- also proposed here: Add GitHub action to check links opentelemetry-go-instrumentation#11
In opentelemetry.io it looks like it checks hyperlinks in HTML using htmltest
: https://github.com/open-telemetry/opentelemetry.io/blob/9fb34a099bd99fd37dc526cc3a6c09e7d037fd15/Makefile#L19
Some OTel repos also use https://github.com/tcort/markdown-link-check (AFAIK it is used in otel-spec, otel-demo and otel-collector).
Personally, I prefer https://github.com/lycheeverse/lychee as:
- it can be used not only for markdown
- you can run it "natively" (created in Rust so you can just grab a binary)
- it is under a GitHub organization (not a user)
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@cijothomas , I think that it is worth at least to merge link fixes if you prefer not to check links every build. Should I remove CI-link validation from this PR? |
@cijothomas, @utpilla - changes related to the ci-link validation are reverted. We can revisit it later. For now, I would like to merge only fixed links. |
Fixes # N/A
Changes
Propagate changes from open-telemetry/opentelemetry-dotnet-instrumentation#1503Fix links.