Add post-tag-version-bump to freeze default_version after a release - #95
jnasbyupgrade wants to merge 5 commits into
Conversation
Committing versioned SQL files (sql/{ext}--{version}.sql) means ongoing
development after a release can silently regenerate and overwrite the file
that was just released, since `make` always regenerates whatever file
matches the current default_version. New target `post-tag-version-bump`
bumps each extension's default_version to a placeholder alias (`stable` by
default) via the new `bump-default-version.sh` script, so a subsequent
`make` freezes the released file instead of overwriting it.
Deliberately a separate, explicit step rather than wired into `tag`/`dist`:
both of those run routinely outside of an actual release (including from
this project's own test suite), and `dist` is documented/tested to leave
the repository clean -- auto-bumping on every such run would both break
that guarantee and risk bumping default_version on a version nobody meant
to release yet.
Controlled via two new variables, following the existing
PGXNTOOL_ENABLE_*/PGXNTOOL_* override pattern:
- PGXNTOOL_ENABLE_POST_TAG_VERSION_BUMP (default yes) makes the target a
no-op when set to no
- PGXNTOOL_POST_TAG_VERSION (default stable) controls the placeholder value
_.gitignore now ignores sql/*--stable.sql to match the default placeholder.
Fixes Postgres-Extensions#20.
Related changes in pgxntool-test:
- Add test/standard/tag-version-bump.bats: standalone script-logic coverage
for bump-default-version.sh, plus make -n dry-run and stub-based coverage
of post-tag-version-bump's wiring, and a real end-to-end smoke test
- Add pgxntool/bump-default-version.sh to the exact distribution-contents
manifest (test/lib/dist-expected-files.txt)
Co-Authored-By: Claude <noreply@anthropic.com>
|
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: Advanced 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 |
…ersion-bump Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merging master's PGXNTOOL_CONTROL_FILES -> _PGXNTOOL_CONTROL_FILES rename (Postgres-Extensions#93) left this recipe referencing the old, now-undefined name, so it invoked the bump script with no control files at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jnasbyupgrade
left a comment
jnasbyupgrade
left a comment
There was a problem hiding this comment.
Ok, overall I think we're missing something here. For one thing, there's no discussion about setting the new version number as part of doing an actual release.
Do we have any existing docs about what the intended release process is? If not, lets start there; I think fleshing that out will make it clearer what we should actually be doing with this particular PR.
| # explicit step in your own release process, right after the tag you're | ||
| # actually releasing has been created and pushed. | ||
| # | ||
| # Variable: PGXNTOOL_ENABLE_POST_TAG_VERSION_BUMP |
There was a problem hiding this comment.
Hmm... as long as this is not wired into anything (ie, won't run unless explicitly called), we don't need a way to disable it.
There was a problem hiding this comment.
Claude here — agreed, removed the PGXNTOOL_ENABLE_POST_TAG_VERSION_BUMP toggle entirely (base.mk, README.asc, HISTORY.asc) and dropped the corresponding disable-case test coverage in the paired pgxntool-test PR. Since it's never auto-invoked, not calling the target already is the opt-out. Pushed as 4ec01a1.
| # everything else on that line (e.g. a trailing comment) and leaving the rest of | ||
| # the file untouched. | ||
| # | ||
| # Invoked by `make tag` (see base.mk) right after a new release tag is created, |
There was a problem hiding this comment.
Uh, is it? This contradicts what the makefile says.
There was a problem hiding this comment.
Claude here — you're right, that comment was wrong (contradicted base.mk's own description of the target as deliberately not wired into tag). Fixed to say make post-tag-version-bump. Pushed as 4ec01a1.
post-tag-version-bump only ever runs when explicitly invoked (never wired into tag/dist), so a PGXNTOOL_ENABLE_* opt-out is pointless -- not calling the target is already the opt-out. Removes the variable, its validation, and the ifeq-gated no-op branch, and the matching README/HISTORY mentions. bump-default-version.sh's header also claimed it's invoked by `make tag`, which contradicts base.mk's own comment describing the target as deliberately NOT wired into tag/dist. Addresses review comments on PR Postgres-Extensions#95. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude here — re the top-level review comment about the release process not covering the new version number: agreed this needs a real answer, not a guess on my part, so I'm not picking an interpretation and building on it. One relevant thing I can report factually rather than judge: Leaving this open rather than resolving it inline — it's a design/process question, not something I should settle unilaterally. |
Consumer-facing release checklist (drafted and reviewed separately on docs/consumer-release-checklist, commit 98083b6), landed directly in this PR so it answers the open review question about where the new version number -- and post-tag-version-bump specifically -- fits into a release. Added a step naming post-tag-version-bump at the point base.mk's own comment says to run it: right after the tag is created and pushed, distinguishing it from the release-version bump earlier in the checklist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary
make post-tag-version-bump, which bumps each extension'sdefault_versionto a placeholder alias (stableby default, via the newbump-default-version.shscript) so ongoing development after a release doesn't silently regenerate and overwrite the just-released version's SQL file.tag/dist, since both of those run routinely outside of an actual release (including in this project's own test suite) anddistis documented/tested to leave the repository clean.PGXNTOOL_ENABLE_*/PGXNTOOL_*pattern:PGXNTOOL_ENABLE_POST_TAG_VERSION_BUMP(defaultyes) andPGXNTOOL_POST_TAG_VERSION(defaultstable)._.gitignorenow ignoressql/*--stable.sqlto match the default placeholder.Fixes #20.
Related pgxntool-test PR: Postgres-Extensions/pgxntool-test#74
Test plan
bump-default-version.shsanity-tested standalone against single/double-quoted control files, trailing comments, multiple files, and error casestest-allsuite: 256/256 passed, 0 skipped🤖 Generated with Claude Code