-
-
Couldn't load subscription status.
- Fork 638
Add automated markdown link checking GitHub Action #1800
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
Conversation
WalkthroughAdds a Markdown link-check configuration and GitHub Actions workflow, and updates many documentation links, paths, anchors, and small text/formatting items across docs. No production code or public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer / Schedule
participant GH as GitHub Actions
participant Repo as Repository (docs/)
participant Action as markdown-link-check
participant Web as External URLs
Dev->>GH: Trigger (push/PR/schedule/dispatch)
GH->>Repo: Checkout repository
GH->>Action: Run with config `.github/markdown-link-check-config.json`
Note over Action: Uses ignore/replacement patterns,\nheaders, timeout, and retry settings
Action->>Repo: Scan `docs/*.md` (max-depth -1)
loop For each URL
Action->>Web: HTTP request (with Accept header for some hosts)
alt 200 or 206
Action-->>GH: Mark link OK
else 429 and retryOn429
Action->>Web: Retry up to retryCount (with fallback delay)
else Error
Action-->>GH: Report broken link
end
end
GH-->>Dev: Job summary (pass/fail)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review for PR #1800: Add automated markdown link checking✅ Approved with CommendationsThis is an excellent addition to the project's quality assurance tooling! 🎯 Strengths
💡 Potential Future EnhancementsConsider adding:
📊 Overall AssessmentThis is a valuable addition that will help maintain documentation quality over time. The configuration is production-ready and follows GitHub Actions best practices. Ready to merge! This will help prevent issues like the one fixed in PR #1799. Great initiative! 🎉 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (11)
.github/markdown-link-check-config.json (4)
15-17: Avoid hardcoding default branch; make replacement resilient.Hardcoding
masterwill break if/when the default branch changes.Apply:
- "replacement": "https://github.com/shakacode/react_on_rails/blob/master/" + "replacement": "https://github.com/shakacode/react_on_rails/blob/${{ github.event.repository.default_branch || 'master' }}/"If expressions aren’t supported here (likely), document the assumption in a comment and mirror the value from a single source in the workflow (see YAML suggestion to set an env var and reuse it).
3-12: Broaden localhost ignores.Add common local hosts to reduce false failures (127.0.0.1, 0.0.0.0, ::1).
"ignorePatterns": [ { "pattern": "^http://localhost" }, { "pattern": "^https://localhost" }, + { + "pattern": "^https?://(127\\.0\\.0\\.1|0\\.0\\.0\\.0|\\[?::1\\]?)" + }, { "pattern": "^mailto:" } ],
21-25: Consider setting a User-Agent for hosts that block bots.Some sites (MDN, Medium, certain docs portals) return 403/429 without a UA. Add a generic UA header to reduce flakes.
"httpHeaders": [ { "urls": ["https://docs.github.com", "https://github.com"], "headers": { - "Accept": "text/html" + "Accept": "text/html", + "User-Agent": "markdown-link-check/ci" } } ],
27-31: Retry only on 429 may miss transient 5xx; consider modest generic retries.If the tool supports it, add limited retries for 502/503/504 to combat flaky hosts on the weekly run.
.github/workflows/check-markdown-links.yml (7)
7-12: Path filters: match intent and avoid extra CI runs.
- Use
docs/**/*.mdto trigger only when docs change (PR objective).- Prefer the canonical
**/*.mdform if you keep repo-wide matching.push: branches: [master] paths: - - '**.md' + - 'docs/**/*.md' - '.github/workflows/check-markdown-links.yml' pull_request: paths: - - '**.md' + - 'docs/**/*.md' - '.github/workflows/check-markdown-links.yml'
1-2: Limit token scope explicitly.Set minimal permissions for the workflow.
name: Check Markdown Links + +permissions: + contents: read
19-21: Add a job timeout.Prevent hung runs from blocking CI.
jobs: markdown-link-check: runs-on: ubuntu-latest + timeout-minutes: 10
22-25: Pin actions to a commit SHA for supply‑chain safety.Pin both actions to a trusted commit (or at least a specific version) to avoid surprise changes.
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + # consider pinning: actions/checkout@<commit-sha> - name: Check markdown links - uses: gaurav-nelson/github-action-markdown-link-check@v1 + uses: gaurav-nelson/github-action-markdown-link-check@v1 + # consider pinning: gaurav-nelson/github-action-markdown-link-check@<commit-sha>
27-34: Speed up PRs: check only modified files on PRs, full scan otherwise.Use expressions to toggle scope and DRY the base branch.
with: use-quiet-mode: 'yes' use-verbose-mode: 'no' config-file: '.github/markdown-link-check-config.json' folder-path: 'docs/' file-extension: '.md' max-depth: -1 - check-modified-files-only: 'no' - base-branch: 'master' + check-modified-files-only: ${{ github.event_name == 'pull_request' && 'yes' || 'no' }} + base-branch: ${{ github.base_ref || 'master' }}
24-35: Pass GITHUB_TOKEN to reduce GitHub rate limits.Expose the default token to the action environment.
- name: Check markdown links uses: gaurav-nelson/github-action-markdown-link-check@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with:
14-16: Optional: make scheduled runs non-blocking.If link rot causes flakes, allow scheduled runs to report but not fail the branch.
schedule: # Run weekly on Monday at 8am UTC - cron: '0 8 * * 1' + # consider a separate job with `continue-on-error: true` for the scheduled event
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/markdown-link-check-config.json(1 hunks).github/workflows/check-markdown-links.yml(1 hunks)docs/react-on-rails-pro/react-on-rails-pro.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml,css,scss,html,graphql,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all formatting; never manually format code
Files:
docs/react-on-rails-pro/react-on-rails-pro.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: claude-review
- GitHub Check: markdown-link-check
- GitHub Check: rspec-package-tests (3.4, minimum)
🔇 Additional comments (1)
docs/react-on-rails-pro/react-on-rails-pro.md (1)
25-25: Approve: relative link target existsdocs/react-on-rails-pro/major-performance-breakthroughs-upgrade-guide.md exists (case matches); relative link is correct.
Implements automated link checking to prevent broken documentation links. Uses markdown-link-check action that runs on: - Push to master (when .md files change) - Pull requests (when .md files change) - Weekly schedule (Mondays at 8am UTC) - Manual workflow dispatch Includes configuration to: - Ignore localhost and mailto links - Handle GitHub relative links properly - Retry on rate limiting - Check all markdown files in docs/ folder This addresses the need for automated 404 detection in documentation as requested by Justin. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
2333a6e to
1fdda76
Compare
Code Review for PR #1800: Add automated markdown link checking GitHub Action🎯 Overall AssessmentThis PR successfully implements automated markdown link checking as requested by Justin to prevent broken documentation links. The implementation is well-structured and addresses the specific issue mentioned in PR #1799. ✅ Strengths
🔍 Suggestions for Improvement
|
Updates moved file paths and removes references to deleted files.
Code Review for PR #1800: Add automated markdown link checking✅ Strengths
🔍 Areas for Improvement
🐛 Potential Issues
💡 Suggestions
✅ Security & Performance
📊 Test CoverageThe workflow itself serves as continuous testing for documentation links. No additional tests are needed for this CI addition. VerdictApproved with minor suggestions ✅ This is a valuable addition that will prevent future documentation issues. Please address the CHANGELOG update and consider the suggestions above before merging. The Gemfile.lock change should be clarified or reverted if unintentional. Great work on improving the documentation reliability! 🎉 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/javascript/images.md (1)
34-34: Fix GitHub URL: useblobor a relative linkVerified: spec/dummy/client/app/startup/ImageExample.jsx exists on the default branch (master); spec/dummy/client/app/packs/ImageExample.jsx not found — update the docs link accordingly.
Apply one of the following diffs (preferred: relative):
- A full example can be found at [spec/dummy/client/app/startup/ImageExample.jsx](https://github.com/shakacode/react_on_rails/tree/master/spec/dummy/client/app/startup/ImageExample.jsx) + A full example can be found at [spec/dummy/client/app/startup/ImageExample.jsx](../../spec/dummy/client/app/startup/ImageExample.jsx)Alternative (absolute, but correct):
- A full example can be found at [spec/dummy/client/app/startup/ImageExample.jsx](https://github.com/shakacode/react_on_rails/tree/master/spec/dummy/client/app/startup/ImageExample.jsx) + A full example can be found at [spec/dummy/client/app/startup/ImageExample.jsx](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/startup/ImageExample.jsx)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/api/view-helpers-api.md(1 hunks)docs/javascript/code-splitting.md(1 hunks)docs/javascript/images.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/javascript/code-splitting.md
- docs/api/view-helpers-api.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml,css,scss,html,graphql,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all formatting; never manually format code
Files:
docs/javascript/images.md
🧠 Learnings (1)
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
docs/javascript/images.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: build
- GitHub Check: markdown-link-check
Updates links to moved config files, eslintrc location, ReactOnRails source, and fixes shakapacker URL typo.
Code Review for PR #1800: Add automated markdown link checking GitHub ActionOverall AssessmentThis is a well-implemented solution for automated link checking that will help maintain documentation quality. The implementation follows GitHub Actions best practices and includes thoughtful configuration to minimize false positives. Strengths
Suggestions for Improvement
Minor Issues
Security: No concerns identifiedTest Coverage: Workflow provides continuous testingPerformance: Well-optimized configurationRecommendationApproved with minor suggestions. This is a valuable addition that will prevent broken documentation links. Great work! |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/additional-details/manual-installation-overview.md (1)
16-16: Tighten wording, code-format API calls, and use GitHub blob URL (not tree).Also align wording with TS usage elsewhere in this PR.
-1. Your JavaScript code "registers" any components and stores per the ReactOnRails APIs of ReactOnRails.register(components) and ReactOnRails.registerStore(stores). See [our JavaScript API docs](../api/javascript-api.md) and the [React on Rails source](https://github.com/shakacode/react_on_rails/tree/master/node_package/src/ReactOnRails.client.ts). +1. Your JavaScript/TypeScript code registers components and stores via the ReactOnRails APIs: `ReactOnRails.register(components)` and `ReactOnRails.registerStore(stores)`. See [our JavaScript API docs](../api/javascript-api.md) and the [React on Rails source](https://github.com/shakacode/react_on_rails/blob/master/node_package/src/ReactOnRails.client.ts).If you expect the file path to move again, consider pinning to a commit permalink to avoid link rot.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/additional-details/manual-installation-overview.md(1 hunks)docs/additional-details/upgrade-webpacker-v3-to-v4.md(1 hunks)docs/contributor-info/linters.md(1 hunks)docs/guides/webpack-configuration.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/contributor-info/linters.md
- docs/additional-details/upgrade-webpacker-v3-to-v4.md
- docs/guides/webpack-configuration.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml,css,scss,html,graphql,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all formatting; never manually format code
Files:
docs/additional-details/manual-installation-overview.md
🧠 Learnings (3)
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
docs/additional-details/manual-installation-overview.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/additional-details/manual-installation-overview.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/additional-details/manual-installation-overview.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: markdown-link-check
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: claude-review
The referenced config files no longer exist in React on Rails. Preserves historical context while directing users to current resources.
Code Review for PR #1800: Add automated markdown link checking GitHub Action✅ Overall AssessmentThis is a valuable addition that implements automated markdown link checking as requested. The implementation is solid and addresses the issue of broken documentation links proactively. 🎯 Strengths
🔍 Areas for Improvement1. Missing newline at end of files
|
| branches: [master] | ||
| paths: | ||
| - '**.md' | ||
| - '.github/workflows/check-markdown-links.yml' |
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.
The config file .github/markdown-link-check-config.json should be included as well, since changing it can affect whether the check passes. I initially thought this file shouldn't be included, but the options starting on line 26 do matter.
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Check markdown links | ||
| uses: gaurav-nelson/github-action-markdown-link-check@v1 |
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.
Why this one? It's deprecated and recommends either https://github.com/tcort/github-action-markdown-link-check or https://github.com/UmbrellaDocs/action-linkspector instead.
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.
Separately from the above, consider pinning to the exact SHA, even if the current workflows don't do that yet https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
This is a much less used action than e.g. checkout, so less likely to be a target, but if it is compromised, less likely to be noticed as well.
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.
Why this one? It's deprecated and recommends either https://github.com/tcort/github-action-markdown-link-check or https://github.com/UmbrellaDocs/action-linkspector instead.
Should we consider lychee-action instead? It's more actively maintained and Rust-based. Investigated the config migration:
- Keeps: timeout, retries, headers, status codes
- Loses:
replacementPatterns- but we have zero/links in docs, so it's dead config - Gains: caching, better performance
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.
Your choice, but looks good.
Uses tcort/github-action-markdown-link-check which is actively maintained. Same configuration and API as the deprecated gaurav-nelson version.
Changes to .github/markdown-link-check-config.json now trigger the workflow since config changes affect link checking behavior.
Code Review for PR #1800: Add automated markdown link checking GitHub Action✅ Strengths1. Well-Configured Link Checker
2. Thoughtful Triggers
3. Documentation Improvements
🔍 Issues & Suggestions1. Version Pinning (Security Best Practice) 2. Missing Paths in Workflow Triggers 3. Configuration Enhancement Suggestions 4. Missing CHANGELOG Entry 💡 Additional Recommendations
✅ Overall AssessmentThis is a solid implementation that will help maintain documentation quality. The changes are well-thought-out, fixing existing broken links while preventing future issues. The workflow configuration is sensible with good defaults. Recommendation: Approve with minor suggestions - The PR can be merged as-is, but implementing the version pinning and path alignment suggestions would improve security and consistency. Great work on implementing this automated link checking! This will significantly help prevent documentation decay. |
Uses tcort/github-action-markdown-link-check@a800ad5f (v1.1.0) instead of mutable version tag to prevent supply chain attacks.
Code Review for PR #1800: Add automated markdown link checking GitHub ActionOverall Assessment ✅This is a well-implemented solution that addresses the broken documentation links issue mentioned in #1799. The implementation follows GitHub Actions best practices and includes thoughtful configuration. Strengths 👍
Code Quality ✨
Performance Considerations ⚡
Security 🔒
Suggestions for Improvement 💡
Test CoverageThe workflow itself serves as continuous testing for documentation links. No additional tests needed for this infrastructure change. Minor Notes 📝
Verdict ✅This PR is ready to merge. It provides valuable automation for maintaining documentation quality and prevents future broken links. The implementation is solid, secure, and follows best practices. |
…e bot-blocked domains - Add NPM domains (npmjs.org/com) to ignore list due to 403 bot protection - Add hichee.com to ignore list due to Cloudflare bot blocking - Keep c9users.io as example placeholder URL to ignore 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change all http://reactrails.com URLs to https://reactrails.com - Site automatically redirects HTTP to HTTPS anyway, but HTTPS is cleaner - No functionality change, just avoiding unnecessary redirect 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace dead chlg.co domain with official Git coding standards URL - Fix broken anchor #missing-routes-file-error in troubleshooting guide - Update broken shakacode.com link to point to local documentation - Add bundle-splitting-hello-world.png image for lightweight bundle example - Add outdated/historical links to ignore list (webpacker configs from old tutorial) Fixes 5 out of 6 broken links. Only remaining issues are intentionally preserved historical content in outdated documentation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Actionable comments posted: 4
🧹 Nitpick comments (4)
.github/workflows/check-documentation-links.yml (4)
6-10: Tighten path filters to only run when docs/*.md changes (and ensure nested files match).Current pattern may match more than intended and can be ambiguous. Since the job scans only docs/, filter to docs/**/*.md for both push and PR.
push: branches: [master] paths: - - '**.md' + - 'docs/**/*.md' - '.github/workflows/check-documentation-links.yml' - '.lycheeignore' - 'lychee.toml' pull_request: paths: - - '**.md' + - 'docs/**/*.md' - '.github/workflows/check-documentation-links.yml' - '.lycheeignore' - 'lychee.toml'If you do intend to trigger on any Markdown change repo‑wide, keep the broader filter but prefer the explicit pattern:
- - '**.md' + - '**/*.md'Also applies to: 13-16
24-24: Bound job time with an explicit timeout.Prevents hung link checks from consuming runner minutes.
check-links: runs-on: ubuntu-latest + timeout-minutes: 20
33-34: Improve cache key to reuse across commits and bust on config changes.Keying on SHA defeats reuse; include OS and hash of lychee config instead, with a sensible restore key.
- key: cache-links-${{ github.sha }} - restore-keys: cache-links- + key: cache-links-${{ runner.os }}-${{ hashFiles('lychee.toml', '.lycheeignore') }} + restore-keys: | + cache-links-${{ runner.os }}- + cache-links-
39-45: Optional: make logs quieter and runs steadier.If logs are noisy or rate‑limits bite, consider adding:
- --no-progress to reduce log spam
- --max-concurrency 4 to ease rate limiting (unless already set in lychee.toml)
args: >- --config lychee.toml --base . --cache --max-cache-age 1d + --no-progress docs/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/check-documentation-links.yml(1 hunks).lycheeignore(1 hunks)lychee.toml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .lycheeignore
- lychee.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: markdown-link-check
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/check-documentation-links.yml (1)
1-47: Overall: solid, useful workflow.Nice addition: correct triggers, weekly schedule, caching, and scoping to docs/ with Lychee config. With the security hardening and small tweaks above, this is ready.
526ecc9 to
c671785
Compare
| "pattern": "^https://hichee\\.com" | ||
| }, | ||
| { | ||
| "pattern": "^https://github\\.com/shakacode/react-webpack-rails-tutorial/blob/master/config/webpacker\\.yml$" |
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.
Why do you make the tool ignore URLs that point to the "react-webpack-rails-tutorial" repo?
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.
This change is only for the webpacker.yml file, because it's not in the current master branch of react-webpack-rails-tutorial, so it will cause a 404 error when the checker runs. We should keep it for people upgrading, so I'm ignoring it to avoid triggering the checker workflow. Am I missing something?
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.
You can update links to refer for files from the commit at which these files are created
| # Upgrading rails/webpacker v3.5 to v4 (Outdated) | ||
|
|
||
| The following steps can be followed to update a Webpacker v3.5 app to v4. | ||
| _Note: This document is outdated. The configuration files referenced below were removed from React on Rails. For current configuration, see the [install generator templates](https://github.com/shakacode/react_on_rails/tree/master/lib/generators/react_on_rails/templates) or consider upgrading to [Shakapacker](https://github.com/shakacode/shakapacker)._ |
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.
I don't understand this line. Can it be clearer?
| 1. Merge changes from the new default [.babelrc](https://github.com/shakacode/react_on_rails/tree/master/lib/install/config/.babelrc) to your `/.babelrc`. If you are using React, you need to add `"@babel/preset-react"`, to the list of `presets`. | ||
| 1. Copy the file [.browserslistrc](https://github.com/shakacode/react_on_rails/tree/master/lib/install/config/.browserslistrc) to `/`. | ||
| 1. Merge any differences between [config/webpacker.yml](https://github.com/shakacode/react_on_rails/tree/master/lib/install/config/webpacker.yml) and your `/config/webpacker.yml`. | ||
| 1. Merge changes from the new default `.babelrc` to your `/.babelrc`. If you are using React, you need to add `"@babel/preset-react"`, to the list of `presets`. |
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.
Can you leave links to the files you are mentioning? If the links you have removed are outdated, can you put links to the new files?
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.
If you mean copying these files from this directory, leave links for files inside this directory
https://github.com/shakacode/react_on_rails/tree/master/lib/generators/react_on_rails/templates/redux/base/app/javascript/bundles/HelloWorld
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.
can you put links to the new files?
we don't have new files that are equivalent to the files here, they're similar, but will cause confusion. I'm considering leaving the broken links as is, and just make the file as outdated and what they should do instead.
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.
Good check added, I just left some tiny comments
Code Review for PR #1800: Add automated markdown link checking GitHub ActionSummaryThis PR adds valuable automated link checking functionality to prevent broken documentation links. The implementation is clean, well-configured, and addresses the issue raised by Justin regarding 404 detection. ✅ Strengths1. GitHub Actions Workflow
2. Configuration
3. Documentation Updates
🔍 Observations & Suggestions1. Pinned Action Version 2. Ignore Patterns
Consider adding a comment in the config file explaining why certain URLs are ignored (e.g., "hichee.com - historical reference, no longer active"). 3. Documentation Quality
4. Test Coverage
|
Code Review for PR #1800: Add automated markdown link checking GitHub ActionThank you for implementing this automated markdown link checking solution! This is a valuable addition that will help maintain documentation quality. Here is my comprehensive review: ✅ Strengths
🔍 Suggestions for Improvement1. Performance & Resource UsageThe workflow checks ALL markdown files in docs/ on every run. Consider:
2. Missing Pattern CoverageAdd these patterns to ignorePatterns: {
"pattern": "^#"
},
{
"pattern": "^https://github\\.com/.*#L[0-9]+"
}This handles anchor links and GitHub line number references. 3. Security ConsiderationThe workflow uses a pinned commit SHA for the action, which is great for security. However, consider adding: permissions:
contents: read
pull-requests: writeThis follows the principle of least privilege. 4. Error Reporting EnhancementConsider adding a step to create an issue or notify maintainers when the scheduled run finds broken links: - name: Create issue on failure
if: failure() && github.event_name == "schedule"
uses: actions/github-script@v7
with:
script: |
// Create issue with broken links report5. Configuration DocumentationAdd a comment header to .github/markdown-link-check-config.json explaining:
|
Code Review SummaryThis PR successfully implements automated markdown link checking - a valuable addition for maintaining documentation quality. Strengths• Well-configured GitHub Actions workflow with appropriate triggers Suggestions for Improvement
Security ReviewAll security best practices are followed - action properly pinned, no sensitive data exposed, appropriate permissions. RecommendationApprove with minor suggestions - The implementation is solid and production-ready. Please update the CHANGELOG before merging. Other suggestions can be addressed in follow-up PRs. |
Code Review for PR #1800: Add automated markdown link checking GitHub ActionSummaryThis PR adds valuable automated link checking for markdown documentation to prevent broken links. The implementation is well-structured and follows GitHub Actions best practices. ✅ Strengths
🔍 Suggestions for Improvement
📝 Minor Issues
🔒 Security Considerations
🚀 Performance
✅ Overall AssessmentThis is a well-implemented feature that will significantly improve documentation quality. The automated checks will prevent broken links from being merged and the weekly schedule will catch external link rot. Recommendation: Approve with minor suggestions above, particularly updating the CHANGELOG as mentioned in the PR checklist. Great work on implementing this automated quality check! 🎉 |
Code Review: PR #1800 - Add automated markdown link checking GitHub ActionOverall AssessmentThis PR successfully implements automated markdown link checking to prevent broken documentation links. The implementation is well-structured and follows GitHub Actions best practices. Strengths
Suggestions for Improvement
Minor Issues
Test CoverageWhile this is a CI/CD addition that does not require traditional unit tests, consider:
Additional Recommendations
VerdictAPPROVED with minor suggestions - This is a valuable addition that will significantly improve documentation quality. The implementation is solid, following best practices for GitHub Actions and link validation. Once the CHANGELOG is updated and you have considered the suggestions above, this is ready to merge. Great work on improving the documentation maintenance process! |
Code Review: Automated Markdown Link CheckingThank you for implementing this important feature to prevent broken documentation links! This is a valuable addition that addresses the issue raised about the 404 RSC documentation link. Here's my comprehensive review: ✅ Strengths
🔍 Issues & Suggestions1. Configuration File - Consider More Robust PatternsThe ignore patterns for specific GitHub URLs might become outdated as the repository evolves: {
"pattern": "^https://github\\.com/shakacode/react-webpack-rails-tutorial/blob/master/config/webpacker\\.yml$"
}Consider using more flexible patterns or documenting why these specific files are excluded. 2. Performance ConsiderationThe workflow runs on all markdown files in the
3. Missing Error Handling DocumentationThe workflow should document what happens when links fail. Consider adding:
4. Security Headers ConfigurationThe custom headers for GitHub might need adjustment if GitHub changes their bot detection: "headers": {
"Accept": "text/html"
}Monitor for potential false positives. 📝 Minor Improvements
🐛 Potential Issues
✅ Testing RecommendationsBefore merging:
📈 Overall AssessmentThis is a solid implementation that will significantly improve documentation quality. The changes follow best practices for GitHub Actions, and the configuration is thoughtful. With the minor adjustments suggested above, this will be an excellent addition to the CI/CD pipeline. Recommendation: ✅ Approve with minor suggestions Great work on this implementation! The proactive approach to documentation quality is commendable. 🎉 |
Code Review for PR #1800: Add automated markdown link checking GitHub ActionOverall AssessmentThis PR effectively implements automated markdown link checking to prevent documentation rot and broken links. The implementation is well-thought-out with appropriate configuration for rate limiting, retries, and relevant ignore patterns. The concurrent fix of existing broken links demonstrates the immediate value of this addition. ✅ Strengths
🔧 Suggestions for Improvement
|
Code Review for PR #1800: Add automated markdown link checking GitHub ActionSummaryThis PR adds valuable automated documentation link checking that will prevent broken links in the documentation. The implementation is well-structured and includes comprehensive link fixes across the documentation. Strengths✅ Proactive documentation maintenance: Weekly scheduled runs will catch external link rot Areas for Improvement🔒 Security
⚡ Performance
🛠️ Configuration
Minor Issues
Testing Recommendations
ConclusionThis is a valuable addition that will significantly improve documentation quality. With the security hardening suggestions and minor optimizations above, this will be production-ready. The proactive approach to documentation maintenance aligns well with the project's quality standards outlined in CLAUDE.md. Recommendation: Approve with minor changes for security hardening (action pinning, permissions). |
Code Review for PR #1800: Add automated markdown link checking GitHub ActionGreat work on implementing automated markdown link checking! This will significantly improve documentation quality and prevent broken links from reaching production. Here's my detailed review: ✅ Strengths
🎯 Code Quality & Best PracticesThe implementation follows GitHub Actions best practices:
|
Code Review: Add automated markdown link checking GitHub Action✅ Overall AssessmentThis is a well-implemented addition that addresses the issue of broken documentation links. The implementation is thorough and follows GitHub Actions best practices. 👍 Strengths
🔍 Areas for Consideration
💡 Suggestions for Enhancement
📋 Checklist Compliance
🚀 RecommendationAPPROVED - This is a valuable addition that will help maintain documentation quality. The implementation is solid, and the suggested improvements are minor enhancements rather than blockers. |
Code Review for PR #1800: Add automated markdown link checking✅ Overall AssessmentThis PR successfully implements automated markdown link checking to prevent broken documentation links, addressing the issue mentioned in PR #1799. The implementation is well-structured and production-ready. 🎯 Strengths
🔍 Suggestions for Improvement
|
Summary
Added automated markdown link checking using GitHub Actions to prevent broken documentation links. This implements the 404 detection tool requested by Justin.
Pull Request checklist
Add/update test to cover these changes(Not applicable - workflow addition)Other Information
The workflow will:
This prevents future broken links like the RSC documentation issue fixed in PR #1799.
This change is
Summary by CodeRabbit
Documentation
Chores