feat: added skill to update a repo to the latest rc#132
Conversation
📝 WalkthroughWalkthroughAdds a new "PF Release Candidate Update" skill to the migration plugin: a SKILL.md workflow document and a ChangesPF Release Candidate Update skill addition
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/migration/skills/pf-release-candidate-update/SKILL.md`:
- Around line 17-20: The workflow examples in SKILL.md are inconsistent about
how to invoke latest-release-candidates.sh, which breaks the skill-local path
convention. Update the Step 2 guidance and any other references so they all use
the same $CLAUDE_SKILL_DIR-based path to scripts under
plugins/migration/skills/pf-release-candidate-update, rather than calling bash
latest-release-candidates.sh without a path or implying a copy step.
🪄 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: 520414bd-fd17-42ec-84bf-03f52a59f11a
📒 Files selected for processing (5)
CONTRIBUTING-SKILLS.mdPLUGINS.mdplugins/migration/README.mdplugins/migration/skills/pf-release-candidate-update/SKILL.mdplugins/migration/skills/pf-release-candidate-update/scripts/latest-release-candidates.sh
| - Run from the **target project root** (the repo with `package.json`), not from `ai-helpers`. | ||
| - Network access for `npm view` and package install. | ||
| - The skill script lives at `plugins/migration/skills/pf-release-candidate-update/scripts/latest-release-candidates.sh` in ai-helpers. Copy it into the target project or invoke it by absolute path. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the skill-local script path consistently.
Step 2 calls bash latest-release-candidates.sh with no path, so the workflow only works after an unstated copy step. Point every example at the same $CLAUDE_SKILL_DIR-based path instead.
🛠️ Proposed fix
- The skill script lives at `plugins/migration/skills/pf-release-candidate-update/scripts/latest-release-candidates.sh` in ai-helpers. Copy it into the target project or invoke it by absolute path.
+ The skill script lives in this skill directory. Reference it with `$CLAUDE_SKILL_DIR/scripts/latest-release-candidates.sh`.
@@
- bash latest-release-candidates.sh --update package.json
+ bash "$CLAUDE_SKILL_DIR/scripts/latest-release-candidates.sh" --update package.jsonAs per path instructions, use $CLAUDE_SKILL_DIR to reference files or scripts relative to the skill directory.
Also applies to: 66-68
🤖 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 `@plugins/migration/skills/pf-release-candidate-update/SKILL.md` around lines
17 - 20, The workflow examples in SKILL.md are inconsistent about how to invoke
latest-release-candidates.sh, which breaks the skill-local path convention.
Update the Step 2 guidance and any other references so they all use the same
$CLAUDE_SKILL_DIR-based path to scripts under
plugins/migration/skills/pf-release-candidate-update, rather than calling bash
latest-release-candidates.sh without a path or implying a copy step.
Source: Path instructions
| # ./latest-release-candidates.sh --json # JSON object of package -> version | ||
| # ./latest-release-candidates.sh --update [package.json] # update PF deps in package.json | ||
|
|
||
| set -euo pipefail |
There was a problem hiding this comment.
The fallback path (line 44) and --update mode (line 91) both call node -e, but the script doesn't check for Node.js first. Would a command -v node guard near the top help? CONTRIBUTING-SKILLS.md L234–237 shows the pattern. The SKILL.md would also need a ## Requirements section noting the Node.js dependency.
| ### Diagnosis order | ||
|
|
||
| 1. **Read the error output** — note the first failing file and whether it is a type error, import error, or runtime/test failure. | ||
| 2. **Import paths** — load `pf-import-checker` and fix invalid `@patternfly/*` imports (charts, chatbot, component-groups dynamic paths, missing CSS). |
There was a problem hiding this comment.
These skill names don't match what's on main — pf-import-checker is actually pf-import-check, pf-component-structure is pf-component-check, and pf-class-migration-scanner doesn't exist in the repo. Would describing the capability instead work better here? e.g., "scan for invalid @patternfly/* import paths" — that way the instructions work regardless of which skills or MCP tools the user has installed.
|
|
||
| | Symptom | Likely fix | | ||
| |---------|------------| | ||
| | `Module not found: @patternfly/...` | Wrong import path; see `pf-import-checker` | |
There was a problem hiding this comment.
Same name issue in the common issues table — pf-import-checker should either be the correct name (pf-import-check) or a capability description to stay consistent with the skill name fix above.
| @@ -0,0 +1,154 @@ | |||
| --- | |||
| name: pf-release-candidate-update | |||
| description: >- | |||
There was a problem hiding this comment.
Skill descriptions get loaded into a limited context window alongside every other installed skill — longer descriptions are more likely to get cut off. Would a shorter version keep the trigger words intact? e.g., Update @patternfly/* npm dependencies to the latest release candidate versions. Use when testing the next PF release or bumping to RC packages.
2 participants
Adds a skill to update any project to the latest patternfly rc.
Summary by CodeRabbit