CNTRLPLANE-3677: openshift-developer: add create-pr skill - #569
Conversation
WalkthroughA new ChangesOpenShift Developer skill and metadata
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)plugins/openshift-developer/skills/create-pr/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) Comment |
836aa57 to
bb72e98
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/openshift-developer/.claude-plugin/plugin.json`:
- Line 4: The manifest version in the plugin metadata does not match the PR’s
stated target version. Update the version field in the plugin manifest to the
intended release value, or adjust the PR objective if the lower version is
correct, so the change is consistent with the release plan.
In `@plugins/openshift-developer/skills/create-pr/SKILL.md`:
- Around line 10-12: The fenced code blocks in the skill documentation are
missing language identifiers, which violates the markdown lint rules. Update the
affected fenced blocks in the create-pr SKILL.md content to use the appropriate
identifiers, such as text for generic command examples and bash where the block
is shell syntax. Make the same fix for all referenced blocks so the markdown
stays consistent and lint-compliant.
- Around line 31-35: The create-pr skill still hardcodes the remote name and
conflicts with the remote-discovery guidance. Update the base-branch detection
in create-pr workflow/docs to discover the remote first (for example via git
remote -v or another approved discovery command) and use that value in git
remote show instead of assuming origin. Also revise the surrounding wording in
SKILL.md so the defaults and “do not run” guidance no longer prohibit remote
discovery and instead describe selecting a remote after discovery, keeping the
create-pr instructions aligned with repo policy.
🪄 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: Enterprise
Run ID: 3ffb30df-55b7-4aca-bc0d-cc82a691e19c
📒 Files selected for processing (2)
plugins/openshift-developer/.claude-plugin/plugin.jsonplugins/openshift-developer/skills/create-pr/SKILL.md
| "name": "openshift-developer", | ||
| "description": "Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engineer", | ||
| "version": "1.1.2", | ||
| "version": "1.1.3", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Version bump does not match stated PR objective.
The PR objective says the manifest should be bumped to 1.2.0, but this file sets 1.1.3. Please align either the objective or the manifest version before merge.
🤖 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/openshift-developer/.claude-plugin/plugin.json` at line 4, The
manifest version in the plugin metadata does not match the PR’s stated target
version. Update the version field in the plugin manifest to the intended release
value, or adjust the PR objective if the lower version is correct, so the change
is consistent with the release plan.
Add create-pr skill to create a PR from the current branch for a Jira issue, completing the pre-PR author loop. Bump version to 1.1.3. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bb72e98 to
753a489
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/openshift-developer/skills/create-pr/SKILL.md`:
- Around line 31-35: The remote/base-branch discovery in the PR creation flow is
order-dependent because it uses the first remote, so update the logic in the
create-pr skill to derive the remote from the current branch’s tracking
information first. Adjust the sequence around the remote detection and
BASE_BRANCH setup to use git branch -vv (or equivalent tracking metadata) to
find the branch’s remote, and only fall back to a discovered remote when
tracking info is unavailable; keep the existing BASE_BRANCH resolution flow
intact.
- Line 36: The fallback behavior is documented but not actually implemented: the
create-pr flow should explicitly set the target branch to main when branch
detection fails. Update the command sequence in SKILL.md around the create-pr
instructions so the fallback path in the relevant branch-detection logic uses
main as the default, and make sure the documented behavior matches the actual
steps shown to users.
🪄 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: Enterprise
Run ID: 857f554f-ca33-43e6-aa51-e65e76da9c4b
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/openshift-developer/.claude-plugin/plugin.jsonplugins/openshift-developer/skills/create-pr/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- plugins/openshift-developer/.claude-plugin/plugin.json
| 3. Discover remotes, then determine the base branch: | ||
| ```bash | ||
| REMOTE=$(git remote | head -1) | ||
| BASE_BRANCH=$(git remote show "$REMOTE" | sed -n 's/.*HEAD branch: //p') | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use deterministic remote discovery tied to the current branch
Line 33 picks the first remote, which is order-dependent and can select the wrong base for the PR diff. Prefer discovering from the current branch tracking info (git branch -vv) and only then falling back.
As per coding guidelines, “Code should NEVER assume git remote names like 'origin' or 'upstream' without first discovering them - use git remote -v or git branch -vv to discover remotes first”.
Suggested update
-REMOTE=$(git remote | head -1)
-BASE_BRANCH=$(git remote show "$REMOTE" | sed -n 's/.*HEAD branch: //p')
+REMOTE=$(git branch -vv | awk '/^\*/ {print $0}' | sed -n 's/.*\[\([^\/]*\)\/.*/\1/p')
+[ -z "$REMOTE" ] && REMOTE=$(git remote -v | awk 'NR==1{print $1}')
+BASE_BRANCH=$(git remote show "$REMOTE" | sed -n 's/.*HEAD branch: //p')Also applies to: 104-104
🤖 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/openshift-developer/skills/create-pr/SKILL.md` around lines 31 - 35,
The remote/base-branch discovery in the PR creation flow is order-dependent
because it uses the first remote, so update the logic in the create-pr skill to
derive the remote from the current branch’s tracking information first. Adjust
the sequence around the remote detection and BASE_BRANCH setup to use git branch
-vv (or equivalent tracking metadata) to find the branch’s remote, and only fall
back to a discovered remote when tracking info is unavailable; keep the existing
BASE_BRANCH resolution flow intact.
Source: Coding guidelines
| REMOTE=$(git remote | head -1) | ||
| BASE_BRANCH=$(git remote show "$REMOTE" | sed -n 's/.*HEAD branch: //p') | ||
| ``` | ||
| Fall back to `main` if detection fails. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Implement the documented main fallback explicitly
Line 36 says fallback to main, but the command sequence does not actually set it when detection fails.
Suggested update
BASE_BRANCH=$(git remote show "$REMOTE" | sed -n 's/.*HEAD branch: //p')
+[ -z "$BASE_BRANCH" ] && BASE_BRANCH=main🤖 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/openshift-developer/skills/create-pr/SKILL.md` at line 36, The
fallback behavior is documented but not actually implemented: the create-pr flow
should explicitly set the target branch to main when branch detection fails.
Update the command sequence in SKILL.md around the create-pr instructions so the
fallback path in the relevant branch-detection logic uses main as the default,
and make sure the documented behavior matches the actual steps shown to users.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
create-prskill to the openshift-developer plugin — creates a PR from the current branch for a Jira issue, completing the pre-PR author loop afterjira:solve,code-review:pre-commit-review, andaddress-review-precommit./openshift-developer:create-prinstead of constructing custom prompts inline.Test plan
/openshift-developer:create-pris listed as an available skill/openshift-developer:create-pr <ISSUE_KEY> --upstream <repo> --head <fork>:<branch>🤖 Generated with Claude Code
Summary by CodeRabbit
gh.