feat(dx): add /lfx-pr slash command for PR shipping#672
feat(dx): add /lfx-pr slash command for PR shipping#672manishdixitlfx wants to merge 12 commits into
Conversation
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Changes/lfx-pr Command Guide
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 @.claude/commands/lfx-pr.md:
- Line 40: Replace the second top-level heading text "# Note: -S (GPG signing)
is recommended but not enforced by repo policy. Add it if you have GPG
configured." with a normal sentence (no leading "# ") and remove the trailing
punctuation to avoid MD025 and MD026; update the text in
.claude/commands/lfx-pr.md so it becomes a plain paragraph like the existing
prose rather than an H1.
🪄 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: ad0d904b-3494-4862-a7b8-dd89490bc9a0
📒 Files selected for processing (1)
.claude/commands/lfx-pr.md
There was a problem hiding this comment.
Pull request overview
Adds a new Claude Code slash command (/lfx-pr) intended to standardize and automate the “ship a PR” workflow in the lfx-v2-ui monorepo (preflight checks, optional commit, quality gates, push, and PR creation via gh).
Changes:
- Introduces a new
.claude/commands/lfx-pr.mdcommand definition with a step-by-step PR shipping workflow. - Documents commit subject validation rules, quality gate commands, and a PR body checklist/template.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review comments from @coderabbitai, @copilot-pull-request-reviewer: - frontmatter: corrected description from (DCO+GPG) to (DCO sign-off; GPG optional) to match actual command behavior (per @copilot-pull-request-reviewer) - Step 1: fetch origin/main and use origin/main..HEAD for ahead check; avoids stale-local-main drift (per @copilot-pull-request-reviewer) - Step 2: relaxed scope rule per .claude/rules/commit-workflow.md and delegated final validation to yarn commitlint (per @copilot-pull-request-reviewer) - Step 2: converted "# Note:" pseudo-comment to blockquote and wrapped commit commands in fenced bash block; fixes MD025/MD026 markdownlint failures that were blocking CI (per @coderabbitai) - Step 3: switched yarn lint/format to :check variants so ship-time gates are read-only (no working-tree mutation) (per @copilot-pull-request-reviewer) - Step 4: git diff main...HEAD -> git diff origin/main...HEAD for consistency with origin-based comparisons (per @copilot-pull-request-reviewer) Item 6 (move commit-format/Signed-off-by from advisory to fail-fast) deferred pending user direction — left thread unresolved. Resolves 5 of 6 review threads. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Review Feedback AddressedCommit: c713ead Changes Made
Threads Resolved5 of 6 unresolved threads addressed in this iteration. Still Open
|
|
Tip For best results, initiate chat on the files or code changes.
Regarding the still-open Step 4 item: your proposed split makes good sense — separating CI/hook-enforced rules (commit format, |
Agent-Logs-Url: https://github.com/linuxfoundation/lfx-v2-ui/sessions/fadab2f2-1125-4f1f-a2ea-8f48bdbadf00 Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com>
Address review comments from @jordane: - frontmatter, Step 2 commit cmd, Note, mandatory sentence, Step 4 hard checks, Step 6 PR body checklist, and failure-handling section all updated to make -S (GPG signing) mandatory alongside --signoff (DCO). Reverses the prior "GPG optional" direction per @jordane's confirmation that GPG signing is enforced by repo policy. Step 4's Signed-off-by verification now also checks GPG signature status via `git log --format='%G?'` (per @jordane on line 46) - frontmatter argument-hint, new $ARGUMENTS interpretation paragraph, Step 1 dirty-tree branch, and Step 2 heading reworked so $ARGUMENTS is treated as freeform user instructions (commit subject, target branch, draft toggle, PR body context, etc.) rather than strictly as a commit subject. If no commit subject is supplied, the command proposes one from the staged diff and asks the user to confirm (per @jordane on line 23) T6 (Step 4 hard vs advisory split) was implemented by copilot-swe-agent in 6537f5f; keeping that change as-is. Resolves 3 review threads. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Review Feedback Addressed — Iteration 2Commits in this iteration:
Changes MadeGPG signing now mandatory (per @jordane)
Step 4 split (autofixed by @copilot-swe-agent in 6537f5f, kept as-is)
Threads Resolved3 of 3 unresolved threads addressed in this iteration. Side Notes
|
Address review comments from @copilot-pull-request-reviewer: - Step 4 hard checks: replaced the manual prose Signed-off-by check with a single combined verification command that asserts both GPG signature status (`%G?`) and trailer presence per commit via `git log --format='%G? %(trailers:key=Signed-off-by,valueonly...)'` — fail-fast block is now fully actionable rather than partly manual (per @copilot-pull-request-reviewer on line 75) - Step 6 gh pr create: dropped the branch-name fallback for PR title and clarified the title must be `type(scope): description`, lowercase, no JIRA ticket. If the first commit subject doesn't satisfy that, command asks user for a valid title. Prevents PR-title-lint CI failures from branch-name fallbacks like `feat/LFXV2-1697` (per @copilot-pull-request-reviewer on line 124) - Step 6 PR-body checklist: `yarn lint` -> `yarn lint:check`, added `yarn format:check` line, so checklist labels now match the gates actually run in Step 3 (per @copilot-pull-request-reviewer on line 112) Item 4 (false positive on GPG mandate, line 50) is being handled by companion PR #674 which documents the GPG-enforcement policy in CLAUDE.md and .claude/rules/commit-workflow.md. Item 5 (draft-mode contradiction at line 13) deferred pending user direction on whether to implement --draft end-to-end or remove the draft references from $ARGUMENTS interpretation. Resolves 3 of 5 review threads; 1 reply-only (false positive); 1 deferred. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Review Feedback Addressed — Iteration 3Commit: ee7e63f Changes Made
No Change Needed
Threads Resolved4 of 5 unresolved threads addressed in this iteration. Still Open
Note on Review Status@jordane left CHANGES_REQUESTED at 15:55 UTC, before iteration 2 (commit 576a788 at 17:13) and iteration 3 (this commit). Iteration 2 specifically addressed @jordane's three threads on GPG enforcement, freeform args, and the Step 4 hard/advisory split. Pinging @jordane for a re-review when convenient. |
Review Feedback Addressed — Iteration 3 Wrap-UpCommits in this iteration window:
Final State — All 5 Threads From This Batch Resolved
Note for @jordaneIteration 2 (commit |
Mirrors the verification-snippet relaxation from PR #674 so the /lfx-pr command and the canonical commit-workflow rule stay in sync. - Accept %G? codes G or U (good signature, with U meaning the signing key isn't in the local trust db — fine for policy purposes) - Flag N / B / E as codes needing investigation - Note that GitHub's Verified badge is the authoritative post-push check, since local %G? depends on which keys the user has imported Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
* docs(dx): document GPG signing requirement for commits Surfaced during PR #672 review: @jordane confirmed GPG signing is enforced by repo policy, but neither CLAUDE.md nor .claude/rules/commit-workflow.md mentioned it. Both said only DCO sign-off was required, which caused churn during that review. - CLAUDE.md: updated the single-line commit guidance to `git commit --signoff -S` and noted both signatures are enforced. - .claude/rules/commit-workflow.md: added a "Commit Signing" section with one-time GPG setup, the standard commit command, and a signature-verification snippet for branch validation. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * docs(dx): align contributor docs with GPG-enforcement policy Address PR #674 review feedback from @copilot-pull-request-reviewer and @jordane. - CONTRIBUTING.md (Sign-off section): renamed to "Sign-off and GPG Signing", expanded to require both --signoff and -S, included one-time GPG config, and pointed at .claude/rules/commit-workflow.md as the canonical policy source (per @copilot-pull-request-reviewer) - .claude/agents/code-standards-enforcer.md (General Rules checklist): updated the commit-signing item to require both --signoff and -S with a cross-reference to commit-workflow.md (per @copilot-pull-request-reviewer) - .claude/rules/commit-workflow.md (verification snippet): relaxed acceptable %G? codes to G or U (good-but-untrusted is fine for policy), documented that N/B/E need investigation, and noted that GitHub's Verified badge is the authoritative post-push check since local %G? depends on which keys are in the user's trust db (per @copilot-pull-request-reviewer and @jordane) Resolves 2 review threads. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Co-authored-by: David Deal <ddeal@linuxfoundation.org>
asithade
left a comment
There was a problem hiding this comment.
This PR adds a new Claude Code slash command at .claude/commands/lfx-pr.md that ships a PR end-to-end (pre-flight checks, commit with DCO + GPG, quality gates, self-review, push, open PR). Markdown-only change. The command itself is well-structured, follows the same style as existing skills, and the previous 13 review-comment iterations have addressed the substantive concerns.
Remaining items are minor:
- A few NITs around fenced-block consistency and a duplicate sentence in Step 2.
- One unresolved upstream-policy citation (GPG enforcement) — recommend linking PR #674 inline.
- A reference to
lfx-preflightin the PR-body checklist that does not match any artifact in this repo. - JIRA ticket / branch naming — no LFXV2-XXXX in commits, PR body, or branch. The ticket waiver expired on 2026-04-28; tickets are required again.
Other observations:
- Markdownlint runs clean.
- The PR body itself is bare (just
Signed-off-by:) — ironic for a PR that adds a tool prescribing a detailed PR template.
PR-level findings
SHOULD_FIX — JIRA ticket and branch naming
Rule: .claude/rules/commit-workflow.md § JIRA Tracking + project memory (waiver EXPIRED 2026-04-28)
No JIRA LFXV2-XXXX ticket is referenced in any commit message or in the PR body. The ticket waiver expired on 2026-04-28; tickets are required again. Branch name feat/lfx-pr-command also does not follow <type>/LFXV2-<number>. Create a ticket and reference it in the PR body and a commit body trailer.
NIT — PR body should describe the change
Rule: .claude/rules/commit-workflow.md PR body should describe the change
The PR body currently contains only Signed-off-by:. Since this PR adds a slash command that itself prescribes a rich PR body template, the PR body here should ideally model the same pattern — "eat your own dogfood." Add a one-paragraph summary and link to the JIRA ticket and PR #674.
Verdict: REQUEST_CHANGES
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
.claude/commands/lfx-pr.md:18
git fetch origin mainonly fetchesmainintoFETCH_HEADand can leave theorigin/mainremote-tracking ref stale, while the following ahead/diff/commitlint checks all useorigin/main. Fetch the remote-tracking ref (for example with a plaingit fetch originor an explicitmain:refs/remotes/origin/mainrefspec) before relying onorigin/main.
- git fetch origin main → ensure `origin/main` is current (avoids stale-local-main drift)
| description: Ship a PR for lfx-v2-ui — runs quality gates, signs commits (DCO sign-off + GPG), pushes branch, opens PR with LFXV2 linkage. | ||
| argument-hint: [optional: any extra instructions — commit subject, target branch, draft mode, extra PR body context] | ||
| --- | ||
|
|
||
| # /lfx-pr — Ship a PR for lfx-v2-ui | ||
|
|
||
| You are running in the lfx-v2-ui monorepo. The user is invoking this command to ship the current branch as a PR. Follow these steps in order. Fail fast — stop immediately if any step fails and surface the error. |
|
|
||
| User-provided argument (if any): $ARGUMENTS | ||
|
|
||
| Interpret `$ARGUMENTS` as freeform user instructions. They may include any of: a commit subject (e.g., `feat(auth): add OAuth`), a target branch (e.g., `to feat/lfx-pr-v2`, `move commits to jme/branch-name`), a draft toggle (e.g., `as draft`), additional PR body context, or other adjustments to the default flow. Apply each instruction at the step that owns it (commit subject in Step 2, branch operations in Steps 1/5, draft flag in Step 6). If any directive is ambiguous, ask the user before acting on it. |
| @@ -0,0 +1,167 @@ | |||
| --- | |||
| description: Ship a PR for lfx-v2-ui — runs quality gates, signs commits (DCO sign-off + GPG), pushes branch, opens PR with LFXV2 linkage. | |||
| argument-hint: [optional: any extra instructions — commit subject, target branch, draft mode, extra PR body context] | |||
Address review comments from @copilot-pull-request-reviewer and @asithade: - .claude/commands/lfx-pr.md (signoff/GPG Note): merge the duplicate blockquote + prose into a single line and link to .claude/rules/commit-workflow.md § Commit Signing for the authoritative policy (per @copilot — in-repo citation; per @asithade — dedupe). - .claude/commands/lfx-pr.md (Step 4): fence the bare 'git diff origin/main...HEAD' invocation and rewrite the trailing prose for consistency with surrounding fenced blocks (per @asithade). - .claude/commands/lfx-pr.md (Step 5): fence the bare 'git push -u origin HEAD' and inline-code the rebase suggestion (per @asithade). - .claude/commands/lfx-pr.md (pre-merge checklist): replace the non-existent 'lfx-preflight' skill reference with a direct pointer to the actual enforcement: '.claude/hooks/guard-protected-files.sh' (per @asithade). The thread on '%G?' accepting both 'G' and 'U' was already addressed in the current file (Step 4 explicitly accepts both) — no code change, just reply + resolve. Resolves 7 review threads. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Review Feedback AddressedCommit: Changes Made
No Change Needed
Threads Resolved7 of 7 unresolved threads addressed in this iteration. Follow-up
|
Agent-Logs-Url: https://github.com/linuxfoundation/lfx-self-serve/sessions/e763f1b7-4883-4210-a013-69adc7f4d1f5 Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com>
Agent-Logs-Url: https://github.com/linuxfoundation/lfx-self-serve/sessions/e763f1b7-4883-4210-a013-69adc7f4d1f5 Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com>
Addressed the remaining in-file policy-citation item in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Signed-off-by: Manish Dixit mdixit@linuxfoundation.org