Skip to content

feat: breaking change skill#116

Open
dlabaj wants to merge 1 commit into
patternfly:mainfrom
dlabaj:breaking
Open

feat: breaking change skill#116
dlabaj wants to merge 1 commit into
patternfly:mainfrom
dlabaj:breaking

Conversation

@dlabaj

@dlabaj dlabaj commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Created skill that checks for breaking changes by react.

Summary by CodeRabbit

  • Documentation
    • Added new migration skill for scanning PatternFly React API breaking changes and generating detailed markdown reports
    • Includes comprehensive workflow documentation and standardized report template for identifying and addressing API compatibility issues

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

A new pf-react-breaking-changes skill is added to the migration plugin. The skill includes a 166-line guide (SKILL.md) defining workflow, scope, detection patterns, and a quality checklist, plus a 111-line Markdown report template. The skill is registered in the migration plugin README and the top-level PLUGINS.md.

Changes

pf-react-breaking-changes Skill

Layer / File(s) Summary
Skill guide and report template
plugins/migration/skills/pf-react-breaking-changes/SKILL.md, plugins/migration/skills/pf-react-breaking-changes/references/report-template.md
SKILL.md defines scope rules, a step-by-step workflow using PatternFly MCP/docs and optional pf-codemods dry runs, detection/search patterns, reporting defaults (src/ scan path, pf-react-breaking-changes-report.md output), and a quality checklist. The report template provides all placeholder sections for severity-based and component-based findings, migration steps, exclusions, and reference links.
Skill registration in docs
plugins/migration/README.md, PLUGINS.md
The migration plugin README adds the new skill to the "What's Included" list and updates the file structure diagram. PLUGINS.md adds the corresponding one-line entry under the migration section.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • jpuzz0
🚥 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 'feat: breaking change skill' directly describes the main change: introduction of a new skill for detecting breaking changes in React, which matches the core objective and file modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch breaking

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped anthropics/claude-plugins-official.


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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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-react-breaking-changes/SKILL.md`:
- Around line 1-7: The SKILL.md file frontmatter is missing the required
`disable-model-invocation: true` field. Since this skill writes a markdown
report file to the project root (pf-react-breaking-changes-report.md), it has
filesystem side effects and must declare this flag to prevent AI models from
invoking it programmatically. Add the `disable-model-invocation: true` field to
the YAML frontmatter in the skill definition.
🪄 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: 1d3a40c1-169c-4b76-b68d-25f5323b3a0a

📥 Commits

Reviewing files that changed from the base of the PR and between bd8becd and adbdf6c.

📒 Files selected for processing (4)
  • PLUGINS.md
  • plugins/migration/README.md
  • plugins/migration/skills/pf-react-breaking-changes/SKILL.md
  • plugins/migration/skills/pf-react-breaking-changes/references/report-template.md

Comment on lines +1 to +7
---
name: pf-react-breaking-changes
description: >-
Scan code for @patternfly/react-* API breaking changes and produce a markdown
report. Use when upgrading PatternFly React versions, auditing component API
usage, or checking for removed props, renamed components, or import path changes.
---

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add disable-model-invocation: true to frontmatter — skill has side effects.

This skill writes a markdown report file to the project root (line 119: pf-react-breaking-changes-report.md), which is a filesystem side effect. Without disable-model-invocation: true, an AI model could invoke the skill programmatically without explicit user request. This field is a required PatternFly convention for skills that mutate files or system state. As per coding guidelines and retrieved learnings, skills with side effects must declare this flag.

🛡️ Proposed fix to add `disable-model-invocation: true`
 ---
 name: pf-react-breaking-changes
+disable-model-invocation: true
 description: >-
   Scan code for `@patternfly/react-`* API breaking changes and produce a markdown
   report. Use when upgrading PatternFly React versions, auditing component API
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
---
name: pf-react-breaking-changes
description: >-
Scan code for @patternfly/react-* API breaking changes and produce a markdown
report. Use when upgrading PatternFly React versions, auditing component API
usage, or checking for removed props, renamed components, or import path changes.
---
---
name: pf-react-breaking-changes
disable-model-invocation: true
description: >-
Scan code for `@patternfly/react-`* API breaking changes and produce a markdown
report. Use when upgrading PatternFly React versions, auditing component API
usage, or checking for removed props, renamed components, or import path changes.
---
🤖 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-react-breaking-changes/SKILL.md` around lines 1 -
7, The SKILL.md file frontmatter is missing the required
`disable-model-invocation: true` field. Since this skill writes a markdown
report file to the project root (pf-react-breaking-changes-report.md), it has
filesystem side effects and must declare this flag to prevent AI models from
invoking it programmatically. Add the `disable-model-invocation: true` field to
the YAML frontmatter in the skill definition.

Sources: Coding guidelines, Learnings

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.

1 participant