feat(dx): add /lfx-pr slash command for PR shipping#672
Conversation
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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 |
![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 @.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. |
Agent-Logs-Url: https://github.com/linuxfoundation/lfx-v2-ui/sessions/c8dff872-ee27-49b6-b4de-7cb2e7ff3ffb Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com>
Addressed in |
Restores and completes draft-mode handling that 3d28b99 had removed. The autofix removed the references but didn't address the underlying design choice; per user direction, the right resolution is to wire --draft through end-to-end so opt-in drafts work, rather than dropping the feature. - Frontmatter argument-hint: restored "draft mode" to the example list - Args interpretation: restored "a draft toggle (e.g., as draft)" and the "draft flag in Step 6" routing hint - Step 6 gh pr create: documented that a draft directive in $ARGUMENTS appends --draft to the gh invocation; updated the manual-open fallback note as well - "When to use" section: replaced "Not for drafts" exclusion with "Drafts — opt in via a draft directive in $ARGUMENTS" Default remains a review-ready PR; drafts are explicit opt-in. Resolves the last unresolved thread on PR #672. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
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 |
| git log --format='%G? %(trailers:key=Signed-off-by,valueonly,separator=%x20) %h %s' origin/main..HEAD | ||
| ``` | ||
|
|
||
| Each line must start with `G` (good GPG signature) **and** have a non-empty `Signed-off-by` value before the SHA. If any commit fails either check, stop and report. |
| > **Note:** Both `--signoff` (DCO) and `-S` (GPG signing) are required by repo policy. Configure GPG if you haven't already. | ||
|
|
||
| Both `--signoff` (DCO) and `-S` (GPG signing) are mandatory per repo policy. If either fails, stop — do not push unsigned commits. |
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
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
| ``` | ||
|
|
||
| > **Note:** Both `--signoff` (DCO) and `-S` (GPG signing) are required by repo policy. Configure GPG if you haven't already. | ||
|
|
There was a problem hiding this comment.
NIT — Rule: .claude/rules/development-rules.md § Documentation Maintenance (remove duplicates)
The blockquote at line 47 and the prose at line 49 say nearly the same thing back-to-back about DCO+GPG being mandatory. Consider merging into one statement.
|
|
||
| Then run advisory review on: | ||
|
|
||
| git diff origin/main...HEAD — read it. Check against CLAUDE.md and .claude/rules/: |
There was a problem hiding this comment.
NIT — Rule: Consistency with surrounding Step 4 fenced blocks
git diff origin/main...HEAD — read it. is a bare command in prose, while the two hard-check blocks above are properly fenced. For consistency wrap the diff invocation in a fenced bash block.
| git diff origin/main...HEAD — read it. Check against CLAUDE.md and .claude/rules/: | |
| ```bash | |
| git diff origin/main...HEAD |
Read the diff and check against CLAUDE.md and .claude/rules/:
|
|
||
| ## Step 5: Push | ||
|
|
||
| git push -u origin HEAD |
There was a problem hiding this comment.
NIT — Rule: Fenced-block consistency
git push -u origin HEAD (Step 5) is bare prose. Every other shell invocation in the file is fenced. Wrap this one too.
| git push -u origin HEAD | |
| ```bash | |
| git push -u origin HEAD |
| - [x] DCO sign-off (`--signoff`) and GPG signed (`-S`) on every commit | ||
| - [x] Self-reviewed against `CLAUDE.md` conventions | ||
| - [ ] CodeRabbit review addressed | ||
| - [ ] Code-owner review (required for `lfx-preflight` protected files) |
There was a problem hiding this comment.
SHOULD_FIX — Rule: .claude/rules/commit-workflow.md external references; in-repo accuracy
Code-owner review (required for lfx-preflight protected files) — there is no skill named lfx-preflight in this repo (only /preflight). The actual protected-files enforcement lives in .claude/hooks/guard-protected-files.sh. Either rename to /preflight or reference the hook directly.
| - [ ] Code-owner review (required for `lfx-preflight` protected files) | |
| - [ ] Code-owner review (required for files matched by `.claude/hooks/guard-protected-files.sh`) |
|
|
||
| > **Note:** Both `--signoff` (DCO) and `-S` (GPG signing) are required by repo policy. Configure GPG if you haven't already. | ||
|
|
||
| Both `--signoff` (DCO) and `-S` (GPG signing) are mandatory per repo policy. If either fails, stop — do not push unsigned commits. |
There was a problem hiding this comment.
SHOULD_FIX — Rule: UNRESOLVED Copilot thread
The reviewer's open thread asks for an in-repo citation for the GPG-required policy. Author replies that PR #674 documents it. Recommend: add an inline link to #674 in the Note so the policy is at least traceable from inside this file while #674 is in flight.
Suggested fix: append to the blockquote:
See PR #674 which adds this policy to
CLAUDE.mdand.claude/rules/commit-workflow.md.
Signed-off-by: Manish Dixit mdixit@linuxfoundation.org