Skip to content

Retire /commit; move PR-prep guidance into /pr - #105

Open
jnasbyupgrade wants to merge 2 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:worktree-issue-93-pr-workflow-docs
Open

jnasbyupgrade wants to merge 2 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:worktree-issue-93-pr-workflow-docs

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Retire the /commit skill and move what is still project-specific into /pr (.claude/commands/pr.md). Both repos allow only squash and rebase merges, and with squash merges the PR title and body become master's commit message, so branch commit messages and hashes don't reach master and the paired-repo cross-reference belongs in the PR description, as a PR URL.

  • /pr gains HISTORY.asc handling for pgxntool PRs (entry proposal, plus the "Issues fixed in this release:" line kept in sync with the PR's Fixes #N set, re-checked whenever pushes change it), the advisory debug-level check for pgxntool .sh diffs, a /test PASS-with-zero-skips gate before pushing, and URL-only cross-references placed after the opening paragraph. Step 4 now reads the body and writes it back with --body-file, since gh pr edit has no --add-body flag.
  • CLAUDE.md: the PR-workflow paragraph drops the two-phase hash injection; "Claude Skills and Commands" names /pr; the PR-branch force-push rule (ask literally, every time, including your own draft) moves into "Git Commit Guidelines" unchanged.
  • /crossref-audit points at /pr and CLAUDE.md; the pgxntool-test side is expected to reference pgxntool by PR URL, with a raw hash accepted only for historical master commits. scripts/audit.sh is unchanged.
  • /release: the fixed-issues step describes /pr as the per-PR mechanism and itself as the safety net.

Changes only in pgxntool-test. No related changes in pgxntool.

Fixes #93

🤖 Generated with Claude Code

Commits land on a PR branch, never on master; what reaches master is the
merge result, which for a squash merge is the PR title and body.

- CLAUDE.md, `/commit` skill and its format guide: describe composing
  commits on a PR branch for review. `/commit` now stops and asks if either
  repo is on `master`/`main`. The two-phase hash injection stays, as a
  review-time pointer; the cross-reference that must survive the merge goes
  in the PR description, preferably as a PR URL.
- Force-push rules: master is never an agent's call (sole exception:
  `crossref-audit` Step 4); a PR branch is lower risk but still needs a
  literal ask in this repo pair, a deliberate exception to `../ai/PR.md`.
  The two duplicate statements in the `/commit` skill are merged into one.
- `crossref-audit`: note that with squash merges it audits the PR
  description; fixing a pushed PR commit now means a new commit or asking.
- `/commit` "After Pushing" points at the `/ci` skill instead of raw
  `gh run` commands.

Changes only in pgxntool-test. No related changes in pgxntool.

Fixes Postgres-Extensions#93.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2584157e-ea73-4b71-ba6b-2f58018d5252

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

- Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution)
- **A direct GitHub URL to the pgxntool PR or commit (e.g. `https://github.com/Postgres-Extensions/pgxntool/pull/63`) is an acceptable, and preferred, substitute for a raw commit hash.** A PR link stays valid even if the referenced commit is later rebased or amended (its hash changes, but the PR URL doesn't); a bare hash embedded in already-committed text goes stale silently in that case. Prefer linking the PR once one exists.
- If a raw hash was used and the referenced pgxntool commit is later rebased/amended (changing its hash) before your pgxntool-test PR merges, update the reference to the new hash before merging. This only matters pre-merge — once a commit has landed on master, don't rewrite it to fix a stale reference without explicit instruction from the user; rewriting shared history on a protected branch is a different (and much riskier) class of action than amending your own open PR branch.
- Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution). The hash is a pointer for review only: squash and rebase merges both create new commits, so no branch commit's hash reaches master.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Commit hash no longer makes any sense with PRs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Claude here. Agreed. This guide is deleted along with the /commit skill. /pr now says to cross-reference by PR URL only, not a commit hash, in the PR description (with squash merges that is what reaches master). /crossref-audit accepts a raw hash only for historical master commits.

Comment thread .claude/skills/commit/SKILL.md Outdated
Create git commits across pgxntool and pgxntool-test repositories following project
standards. Preprocesses repo info, runs tests, drafts cross-referenced commit messages,
and executes two-phase commits with hash injection.
Compose git commits on PR branches across pgxntool and pgxntool-test for review,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this skill even make sense now? Have a thinker review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Claude here. A thinker review concluded it doesn't, so the skill is retired and its directory deleted. The project-specific parts moved to /pr: HISTORY.asc handling (now per PR), the debug-level advisory, the /test gate before pushing, and URL cross-references. The PR-branch force-push rule moved, unchanged, to CLAUDE.md "Git Commit Guidelines".

With squash merges (both repos allow only squash and rebase), the PR
title and body become master's commit message, so branch commit
messages and hashes don't reach master and the paired-repo
cross-reference belongs in the PR description as a PR URL.

- Delete .claude/skills/commit/ (skill, format guide, gather script).
- /pr gains HISTORY.asc handling (entry plus the fixed-issues line,
  kept in sync with the PR's Fixes #N set), the debug-level advisory,
  a /test PASS-with-zero-skips gate before pushing, and URL-only
  cross-references placed after the opening paragraph. Step 4 uses
  --body-file, since gh pr edit has no --add-body flag.
- CLAUDE.md drops the two-phase hash injection, names /pr, and holds
  the PR-branch force-push rule unchanged.
- crossref-audit and release point at /pr instead of /commit.
@jnasbyupgrade jnasbyupgrade changed the title Reframe commit guidance around PR branches Retire /commit; move PR-prep guidance into /pr Oct 12, 2026
@jnasbyupgrade
jnasbyupgrade marked this pull request as ready for review October 16, 2026 22:25
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 15s —— View job


Code review: PR #105

Overall the retirement of /commit and the move into /pr is coherent. I found one leftover and one minor point.

  1. Stale /commit permissions in .claude/settings.json (lines 23, 48, 51). These still allow Bash(bash .claude/skills/commit/scripts/*) and .../gather-repo-info.sh:*. The PR deletes that directory, so the entries are dead. Remove them so the allowlist doesn't point at paths that no longer exist.

  2. /pr item 4 vs. the "Changes only in [repo]" line. The single-repo line (Changes only in [repo]. No related changes in [other repo].) is correctly placed after the opening paragraph. The step 4 gh pr create template matches. No change needed, but the numbering in pr.md already had a duplicated "3." before this PR. You could fix it while you're in the file.

Everything else checked out. These were no other references to /commit, commit-message-format, or gather-repo-info in the tree besides settings.json and a wording match in audit.sh, which is fine. /crossref-audit, /release and CLAUDE.md are consistent with each other. The --body-file approach in /pr step 4 is correct, since gh pr edit has no --add-body.

Note: the checkout I reviewed had a dirty working tree holding the pre-PR content. I reviewed HEAD, not the working tree, and made no changes.

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.

CLAUDE.md: commit-centric agentic guidance obsolete under PR workflow

1 participant