release skill: own PGXS-target modifications, pin DEBUG exception, gh-repo-sync preflight - #69
Merged
jnasbyupgrade merged 3 commits intoJul 31, 2026
Conversation
…to-sync preflight (Postgres-Extensions#68) Three related dev-tooling improvements to the `/release` skill and its supporting definitions, all found/decided during 2.3.0 release prep. No changes to pgxntool-test's own test suite or template.
…lumbing Replaces the manual git fetch/merge-base/merge --ff-only/push sequence with 'gh repo sync' -- the CLI equivalent of GitHub's 'Sync fork' button. It's fast-forward-only by default and refuses (non-zero exit) rather than creating a merge commit if a true fast-forward isn't possible, same guarantee as before with less code to maintain. Also fixes a real bug in the prior version: local and the fork (origin) are now checked and synced independently and unconditionally, not just when local was found behind. The previous version only checked/pushed the fork inside the 'local was behind' branch, so if local already matched upstream (e.g. from an earlier session) but the fork was separately stale, it would silently skip fixing the fork. Caught via manual testing: '[ -n "$output" ] && echo ...' silently killed the script under 'set -e' whenever a sync was a no-op (output empty) -- the short-circuited && chain's own exit status is non-zero when the right side doesn't run. Fixed with a proper if/fi block.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
…t -e 'slug=$(fork_slug "$repo_path")' was a bare, unguarded assignment. If fork_slug fails (e.g. no 'origin' remote configured), set -e aborts the whole script immediately -- before errors+=() is ever set, before === Summary ===, before === Remote Names ===. Same bug class as the output-guard fix earlier in this file, just missed on this line. Caught by a background test agent reproducing it in an isolated scratch repo. Low-probability in practice (every real clone here has both origin and upstream configured), but the fix is cheap and closes the gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combines what was previously merged separately as #68 with a follow-up
improvement to the same preflight script, into one PR (master was reset
to before #68 and this replaces it).
CLAUDE.md: pgxntool's own modifications to PGXS-named targets (e.g.installcheck's prerequisites) are now explicitly in-scope for theuser-facing API surface definition, rather than excluded by the old
"Known gap" clause -- this loophole caused two real documentation misses
(a stale README claim about
test's prerequisites, and near-miss onissue #79 during 2.3.0 release prep).
CLAUDE.md: DEBUG's non-documentation is now pinned as a standing,non-flaggable exception, so future
/releaseAPI-doc reviews don'tre-litigate it every release.
.claude/skills/release/scripts/release-preflight.sh: local master andthe fork (
origin) are now synced from upstream usinggh repo sync(the CLI equivalent of GitHub's "Sync fork" button) instead of
hand-rolled git fetch/merge/push plumbing. Both local and fork are
checked and fixed independently and unconditionally (the prior version
only checked the fork when local was also behind, missing the case
where local already matched upstream but the fork was separately
stale). Fast-forward only, never
--force-- fails loudly rather thancreating a merge commit or hard-resetting if a true fast-forward isn't
possible on either side.
[ -n "$output" ] && echo ...silently killed the script underset -ewhenever a sync was ano-op (the common case) -- the short-circuited
&&chain's own exitstatus is non-zero when the right side doesn't run. Fixed with a proper
if/fiblock..claude/skills/release/SKILL.md: prose updated to match, plus a fixfor ambiguous PR-body wording in the release-PR templates that caused a
real mix-up during 2.3.0 (a reviewer briefly mistook the real release PR
for the do-not-merge companion PR).
Test plan
bash -n)(already-in-sync no-op path for both local and fork sync)
reset/recombination