ci(docs): check changed markdown links on pull requests#1139
ci(docs): check changed markdown links on pull requests#113913ernkastel wants to merge 10 commits intoNVIDIA:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a PR-triggered GitHub Actions workflow that runs a local markdown link checker on changed Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GHA as GitHub Actions (docs-links-pr)
participant Script as check-docs.sh
participant FS as Repository File System
participant CI as CI Result
PR->>GHA: open/reopen/synchronize with .md changes
GHA->>GHA: git diff base...head -> list of .md files
alt markdown files present
GHA->>Script: run with --only-links --local-only and file paths
Script->>Script: parse files (skip fenced code), emit line_no<TAB>target
loop per extracted target
Script->>FS: check target exists
FS-->>Script: exists / missing
end
alt missing targets found
Script-->>CI: exit non-zero with "md_path:line_no -> target"
else
Script-->>CI: exit 0
end
else no markdown files
GHA-->>CI: skip link check
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/e2e-cloud-experimental/check-docs.sh (1)
224-230: Handle tilde fences (~~~) in fenced-code skipping.
extract_targetscurrently toggles fence state only for backtick fences, so links inside~~~fenced blocks can still be parsed as real links. Line 225 is the toggle point to broaden.Suggested patch
- if (/^\s*```/) { $in = !$in; next; } + if (/^\s*(```|~~~)/) { $in = !$in; next; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e-cloud-experimental/check-docs.sh` around lines 224 - 230, The fenced-code detection only toggles for backticks (the Perl one-liner in check-docs.sh that currently uses if (/^\s*```/) to flip $in), so add tilde fence support by changing that condition to match either ``` or ~~~ (i.e., update the Perl fence-toggle regex inside the extract/processing one-liner to /^\s*(```|~~~)/ so links inside ~~~ blocks are skipped as well).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-links-pr.yaml:
- Line 35: The current population of md_files captures all changed *.md
including vendored/generated paths; update the mapfile command that sets
md_files to exclude common non-doc directories by filtering out patterns like
node_modules, dist, vendor, build (e.g., modify the git diff pipeline that
produces md_files or append a grep -v -E '^(node_modules|dist|vendor|build)/'
before the sort) so md_files only contains real documentation markdown changes.
---
Nitpick comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 224-230: The fenced-code detection only toggles for backticks (the
Perl one-liner in check-docs.sh that currently uses if (/^\s*```/) to flip $in),
so add tilde fence support by changing that condition to match either ``` or ~~~
(i.e., update the Perl fence-toggle regex inside the extract/processing
one-liner to /^\s*(```|~~~)/ so links inside ~~~ blocks are skipped as well).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47af3454-3672-41b5-a1a6-40f5430603a9
📒 Files selected for processing (3)
.github/workflows/docs-links-pr.yamltest/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
bd5d7fc to
1ff2346
Compare
|
Addressed the actionable review feedback in Updates included:
Validation rerun:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 224-225: Replace the permissive fence-toggle logic in the perl
one-liner (the /^\s*(```|~~~)/ branch that flips $in) with a delimiter-aware
handler: detect fence openings via /^\s*(`{3,}|~{3,})/ and record the fence
character and length into $fch and $flen when entering ($in=1), and only exit
the fence (set $in=0 and clear $fch/$flen) when a closing fence uses the same
character and length >= $flen; update the assignments/tuples that set ($in,
$fch, $flen) accordingly so the link-scanning next/while blocks remain gated by
the refined $in state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed0e22e1-3e2a-47bf-b23d-a297ccdf6b0a
📒 Files selected for processing (3)
.github/workflows/docs-links-pr.yamltest/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/docs-links-pr.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- test/check-docs-links.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 225-235: The fence-closing check using the regex
/^\s*(`{3,}|~{3,})/ incorrectly treats lines like "```not-a-close" as closes;
update the logic in the block that uses the regex and variables ($in, $fch,
$flen) so that a closing fence is only accepted if the matched fence is followed
only by optional whitespace (i.e., nothing else on the line). Concretely, either
change the regex to assert end-of-line or only whitespace after the marker, or
after matching check the remainder of the line for non-whitespace characters
before flipping ($in, $fch, $flen) to close the fence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b784f8ab-e97b-46f5-92cf-76320a8b344d
📒 Files selected for processing (2)
test/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- test/check-docs-links.test.js
Signed-off-by: 13ernkastel <LennonCMJ@live.com>
|
The current head is ready from my side, but the latest required checks are still in GitHub's |
Summary
check-docs.shoutput so broken local links include the source line numberWhy
Issue #552 asks for markdown link checking in CI. The repo already had a useful checker in
test/e2e/e2e-cloud-experimental/check-docs.sh, but it only ran in broader E2E contexts and its broken-link output did not point back to the exact markdown line.This keeps the fix small by reusing the existing checker instead of introducing a second link-checking tool. The pull request workflow runs
--local-onlyon changed markdown files so review-time checks stay fast and avoid flaky network-driven failures.Validation
bash -n test/e2e/e2e-cloud-experimental/check-docs.shruby -e 'require "yaml"; puts YAML.load_file(".github/workflows/docs-links-pr.yaml")["name"]'npx vitest run test/check-docs-links.test.jsbash test/e2e/e2e-cloud-experimental/check-docs.sh --only-links --local-only README.mdCloses #552.
Summary by CodeRabbit
New Features
Tests
Chores
Signed-off-by: 13ernkastel LennonCMJ@live.com