Skip to content

chore: use local rumdl version if exist#10

Merged
teodorus-nathaniel merged 2 commits into
mainfrom
feat/rumdl-local
Jul 6, 2026
Merged

chore: use local rumdl version if exist#10
teodorus-nathaniel merged 2 commits into
mainfrom
feat/rumdl-local

Conversation

@teodorus-nathaniel

@teodorus-nathaniel teodorus-nathaniel commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

resolves #9

Summary by CodeRabbit

  • Bug Fixes
    • Markdown checks now better adapt to project setup, using a local lint tool when available and falling back to a global install when needed.
    • The action now handles changed Markdown files more reliably by choosing the appropriate installation path based on the project’s dependencies.
  • Documentation
    • Updated the action guide to reflect the new Markdown-check behavior and installation flow.

@teodorus-nathaniel teodorus-nathaniel self-assigned this Jul 6, 2026
@holdex

holdex Bot commented Jul 6, 2026

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
teodorus-nathaniel 30min ✅ Submitted Jul 6, 2026, 3:53 PM
georgeciubotaru 5min ✅ Submitted Jul 6, 2026, 3:56 PM

Submit or update total time with:

@holdex pr submit-time 2h

Add time on top of previous submission with:

@holdex pr add-time 1h30m

See available commands to help comply with our Guidelines.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The markdown-check composite action now conditionally installs rumdl as a project dependency (if declared in package.json) or globally as a fallback. Linting resolves a local node_modules/.bin/rumdl binary before falling back to the global installation, erroring if neither exists. Documentation is updated to match.

Changes

rumdl install and resolution

Layer / File(s) Summary
Conditional install and local binary resolution
.github/actions/base/markdown-check/action.yml
Detects rumdl in package.json to choose between installing project dependencies or rumdl globally, and adds a run_rumdl helper that prefers the local binary before falling back to global, erroring if neither is found.
Documentation update
ACTIONS.md
Describes the new conditional installation logic and local-binary preference for the markdown-check action.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: georgeciubotaru

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: preferring the local rumdl version when available.
Linked Issues check ✅ Passed The PR addresses issue #9 by detecting rumdl in package.json and preferring the local binary over a global install.
Out of Scope Changes check ✅ Passed The documentation update and fallback install logic are directly tied to the local rumdl behavior change.
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
  • Commit unit tests in branch feat/rumdl-local

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
.github/actions/base/markdown-check/action.yml (2)

75-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Package-manager switch duplicated.

The pnpm/npm/bun branching here repeats the pattern already used elsewhere (e.g. base/prettier, base/commit-check). Not blocking, but worth extracting to a shared script/composite step if this pattern grows further.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/actions/base/markdown-check/action.yml around lines 75 - 99, The
pnpm/npm/bun branching in the markdown-check action is duplicated across the
install steps and should be consolidated. Update the Install dependencies and
Install rumdl (fallback) logic in action.yml to reuse the same package-manager
handling pattern already used by the shared base actions, preferably by
extracting it into a shared script or composite step so the branching is defined
once and reused.

63-73: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Naive grep can false-positive on non-dependency fields.

grep -q '"rumdl"' package.json matches any quoted occurrence of rumdl, including e.g. a "keywords": ["rumdl"] entry, not just an actual dependency declaration. A false positive routes to the full-deps install path unnecessarily (harmless but wasteful), while a false negative (rumdl declared but not matched due to formatting) silently falls back to global install, diverging from the pinned version — defeating the PR's goal.

Prefer a structured check via node or jq against dependencies/devDependencies keys specifically.

♻️ Proposed fix using node (no extra dependency needed in most runners)
-        if [ -f package.json ] && grep -q '"rumdl"' package.json; then
+        if [ -f package.json ] && node -e '
+          const pkg = require("./package.json");
+          const deps = { ...pkg.dependencies, ...pkg.devDependencies };
+          process.exit(deps.rumdl ? 0 : 1);
+        ' 2>/dev/null; then
           echo "has-rumdl=true" >> "$GITHUB_OUTPUT"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/actions/base/markdown-check/action.yml around lines 63 - 73, The
rumdl detection step in the markdown-check action is using a raw grep on
package.json, which can match unrelated fields and miss real dependency
declarations. Update the check in the Detect rumdl in package.json step to use a
structured lookup with node or jq against the dependencies/devDependencies
entries in package.json, and keep the has-rumdl output tied only to an actual
package dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ACTIONS.md`:
- Around line 83-85: The ACTIONS.md entry has Markdown lines that exceed the
repo’s 80-character limit, triggering MD013. Reflow the three bullet points in
this section so they wrap cleanly under 80 characters while keeping the same
meaning, and preserve the wording around rumdl, package.json, and
./node_modules/.bin/rumdl.

---

Nitpick comments:
In @.github/actions/base/markdown-check/action.yml:
- Around line 75-99: The pnpm/npm/bun branching in the markdown-check action is
duplicated across the install steps and should be consolidated. Update the
Install dependencies and Install rumdl (fallback) logic in action.yml to reuse
the same package-manager handling pattern already used by the shared base
actions, preferably by extracting it into a shared script or composite step so
the branching is defined once and reused.
- Around line 63-73: The rumdl detection step in the markdown-check action is
using a raw grep on package.json, which can match unrelated fields and miss real
dependency declarations. Update the check in the Detect rumdl in package.json
step to use a structured lookup with node or jq against the
dependencies/devDependencies entries in package.json, and keep the has-rumdl
output tied only to an actual package dependency.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5058dadf-219f-48e6-935c-5eb8bfd45c6f

📥 Commits

Reviewing files that changed from the base of the PR and between bbb98f1 and 59124b6.

📒 Files selected for processing (2)
  • .github/actions/base/markdown-check/action.yml
  • ACTIONS.md

Comment thread ACTIONS.md
@teodorus-nathaniel

Copy link
Copy Markdown
Collaborator Author

@holdex pr submit-time 30m

@georgeciubotaru

Copy link
Copy Markdown
Contributor

@holdex pr submit-time 5m

@teodorus-nathaniel teodorus-nathaniel merged commit 1270fed into main Jul 6, 2026
4 checks passed
@teodorus-nathaniel teodorus-nathaniel deleted the feat/rumdl-local branch July 6, 2026 15:58
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.

Problem: rumdl is not following the version in the repo

2 participants