Run markdown link check only on master or when docs change

[justin808]

Summary

• Optimize CI performance by skipping markdown link checks on PRs that don't modify documentation
• Ensure comprehensive link validation on master branch by running on all pushes
• Maintain paths filter on pull_request to only trigger when markdown files change

Changes

• Removed paths filter from push event on master branch (run on all master pushes)
• Kept paths filter on pull_request event (only run when .md files or workflow config changes)
• No changes to weekly schedule or manual workflow dispatch triggers

Impact

• Pull Requests: Link check will only run when markdown files are modified, reducing unnecessary CI runs
• Master Branch: All pushes trigger link check for comprehensive validation
• Scheduled Runs: Weekly runs continue as before

Testing

This PR itself should NOT trigger the markdown link check since it only modifies the workflow YAML file (which is in the paths filter), validating the conditional execution works as expected.

🤖 Generated with Claude Code

---

This change is 

Summary by CodeRabbit

• Chores
  • Updated workflow configuration to trigger on all file changes for push events, while maintaining existing filters for pull request events.

[coderabbitai]

Walkthrough

The push trigger in the markdown link-checking workflow was modified to remove explicit path filters, causing all push events to trigger the workflow. The pull_request trigger maintains its existing path restrictions, creating asymmetric trigger behavior between push and PR workflows.

Changes

Cohort / File(s)	Summary
GitHub Workflows \\.github/workflows/check-markdown-links.yml	Removed path filters from push trigger; pull_request trigger remains restricted to markdown files, workflow file, and config file

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

• Verify that removing push path filters is intentional and doesn't cause unnecessary workflow executions
• Confirm pull_request trigger paths remain appropriate for the workflow's purpose

Suggested reviewers

• Judahmeek

Poem

🐰 A filter stripped, a path set free,
The push events now wild decree!
While PRs keep their careful gate,
The workflow springs at any state. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: optimizing when the markdown link check runs—on every master push but only when docs change in PRs.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/md-link-check-cond

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcb99b7 and de819e1.

📒 Files selected for processing (1)

• .github/workflows/check-markdown-links.yml (0 hunks)

💤 Files with no reviewable changes (1)

• .github/workflows/check-markdown-links.yml

⏰ 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). (3)

• GitHub Check: build
• GitHub Check: markdown-link-check
• GitHub Check: claude-review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review - PR #1933

Summary

This PR optimizes CI performance by removing the paths filter from the push event on the master branch, ensuring all master pushes trigger markdown link validation while keeping the filter on PRs to reduce unnecessary runs.

✅ Positive Aspects

1. Clear optimization strategy: The approach makes sense - comprehensive validation on master while being selective on PRs
2. Well-documented: The PR description clearly explains the rationale and impact
3. Minimal change: Only removes 4 lines, keeping the change focused and easy to understand
4. Consistent with other workflows: Looking at lint-js-and-ruby.yml and main.yml, they use paths-ignore on both push and pull_request events, so having different filtering strategies for different events is a pattern in this repo

🤔 Considerations & Questions

1. Consistency with Repository Patterns

Other workflows in this repo use paths-ignore instead of paths for filtering. For example:

• lint-js-and-ruby.yml:7-8: Uses paths-ignore: ['**.md', 'docs/**'] on push
• main.yml:7-8: Same pattern

Suggestion: Consider whether it would be more consistent to use paths-ignore for non-markdown changes on master rather than running on all pushes. However, your approach of "validate everything on master" is also reasonable and may be preferred for this specific check.

2. CI Cost vs. Benefit Trade-off

Running markdown link checks on every master push (including code-only changes) may increase CI time unnecessarily.

Questions to consider:

• How often are markdown links broken by non-markdown changes?
• Is the weekly scheduled run sufficient to catch stale links?
• Would paths-ignore (inverse approach) be more efficient while still catching issues?

3. PR Description Accuracy

The PR description states:

"This PR itself should NOT trigger the markdown link check since it only modifies the workflow YAML file (which is in the paths filter)"

However, the workflow YAML file IS in the paths filter on line 9:

- '.github/workflows/check-markdown-links.yml'

So this PR WILL trigger the check on the pull_request event. This is actually correct behavior (you want to test workflow changes), but the description seems to expect otherwise.

🎯 Recommendations

Option A: Current Approach (Run on all master pushes)

Keep the PR as-is if:

• Link validation on every master push is a high priority
• CI time/cost is not a concern
• You want maximum confidence that links are always valid

Option B: Use paths-ignore (More selective)

Consider this alternative if CI efficiency is important:

on:
  push:
    branches: [master]
    paths-ignore:
      - 'lib/**'
      - 'spec/**'
      - 'packages/**'
      # ... other code directories

This would skip the check when only code changes, but run it for docs/config changes.

Option C: Hybrid Approach

Keep selective filtering but include key documentation files:

on:
  push:
    branches: [master]
    paths:
      - '**.md'
      - 'docs/**'
      - '.github/workflows/check-markdown-links.yml'
      - '.github/markdown-link-check-config.json'
      - 'README.md'

This is essentially the original behavior, so only choose this if the optimization isn't actually needed.

🔍 Testing & Validation

✅ The change is straightforward and unlikely to cause issues
✅ The weekly schedule provides a safety net
✅ Manual workflow_dispatch allows on-demand testing

Suggestion: After merging, monitor CI runs for a week to ensure:

• The check runs on all master pushes as expected
• The check is skipped on code-only PRs as expected
• No increase in broken links due to missed validations

📊 Final Assessment

Code Quality: ✅ Excellent - clean, minimal change
Best Practices: ⚠️ Differs from repo patterns (but may be intentional)
Potential Bugs: ✅ None identified
Performance: ⚠️ May increase CI time on master
Security: ✅ No concerns
Test Coverage: ✅ Adequate (weekly + manual triggers)

Verdict

The change is technically sound but represents a trade-off decision between comprehensive validation and CI efficiency.

Recommendation: ✅ Approve with monitoring - The approach is valid, but track CI impact and link breakage over the next few weeks to validate the optimization achieves its goals.

---

Review generated with analysis of workflow patterns across the repository