ci: replace the PR-title bump script with release-please - #65
Conversation
The bump script derived the version from the merged PR title and pushed the bump, the tag, the GitHub Release and the Packagist ping with no test gate in front of any of it. release-please keeps the bump in a reviewable Release PR instead, and the suite now runs before the tag. Unlike the other SDKs this repo needs no last-release-sha: its bump commit was pushed to master, so v12.1.0 is reachable and composer.json, src/Constant.php, the tag and Packagist all already agree on 12.1.0. src/Constant.php carries the version behind a // x-release-please-version comment, which is the only anchor the generic updater has. composer.json is handled by the php strategy, which writes a bare semver, so "v12.1.0" becomes "12.1.1" on the next release. Composer accepts both and Packagist derives versions from tags. changelog-sections overrides the php strategy's own default, which makes chore visible and would therefore release on a chore-only window. Every other SDK hides it, and one change landing as a release in php but not in go is the kind of difference nobody remembers. The test job moves into a reusable run_tests.yml that both ci.yml and the release gate call, and ci.yml drops its push trigger, which ran the suite twice per SHA and raced the integration leg against the Stream app several SDK repos share.
mogita
left a comment
mogita
left a comment
There was a problem hiding this comment.
Reviewed at 835d49e. One Should Fix, inline on README.md.
Everything else checks out. The php strategy does update composer.json (RootComposerUpdatePackages), its default sections really do leave chore visible so the changelog-sections override earns its place, v12.1.0 is reachable from master so no last-release-sha is needed, the Packagist credentials are repo-scoped rather than environment-scoped so the packagist job needs no environment: ci, master has no required status checks so the job renames break nothing, and run_tests.yml reproduces the old matrix, lint and integration legs unchanged.
One smaller note left out of the inline comment: resync_tag only reaches echo. The Packagist call posts the repository URL and recrawls every tag, so the README line about setting it to a specific tag to re-announce that tag overstates what it does. getstream-ruby and stream-py name the same input publish_tag, if the naming is worth converging too.
…ut honest RELEASE_SETUP.md described the bump script this PR deletes, as the live flow, and was the only surviving reference to that file. Its How It Works and Creating a Release sections now point at the README; the Packagist account, token and secrets setup stays, since none of that changed. resync_tag took a tag and never used it. The Packagist API posts a repository URL and recrawls every tag, so the input is now a boolean resync_packagist and the README says what it actually does.
1 participant
Ticket
CHA-2963. Fourth repo, after getstream-go, stream-py (#287) and getstream-ruby (#86).
Problem
The bump script derived the version from the merged PR title and then pushed the bump, the tag, the GitHub Release and the Packagist ping with nothing testing the tree first.
Solution
release-please keeps the bump in a reviewable Release PR, and the suite runs before anything irreversible.
release-please-config.json/.release-please-manifest.json, php strategy, manifest seeded at12.1.0.last-release-shahere, unlike the other repos. This workflow pushed its bump commit tomaster, sov12.1.0is reachable andcomposer.json,src/Constant.php, the tag and Packagist already agree on 12.1.0.src/Constant.phpcarries a// x-release-please-versioncomment. The generic updater has no other anchor, and it must stay on the same physical line as the literal.composer.jsonis handled by the php strategy's own updater, which writes a bare semver, so"v12.1.0"becomes"12.1.1". Composer accepts both and Packagist derives versions from tags, so the droppedvis cosmetic.changelog-sectionsoverrides the php strategy's default, which makeschorevisible and would therefore cut a release on a chore-only window. Every other SDK hides it.detectjob finds the merged Release PR waiting to be tagged and the suite runs before the release half.run_tests.ymlthat bothci.ymland the release gate call.ci.ymldrops its push trigger: running on push and pull_request ran the suite twice per SHA and raced the integration leg against the Stream app several SDK repos share.check-pr-titlestays inci.ymland still gates the tests.workflow_dispatchtakesresync_tagto re-announce an existing tag.scripts/release/bump_version.php, and backfills the changelog for 10.1.0 through 12.1.0.How to verify
Dry-run against a throwaway branch carrying one
fix:commit (release-please release-pr --dry-run --trace), now deleted:12.1.1from basev12.1.0composer.jsonv12.1.0->12.1.1src/Constant.php12.1.0->12.1.1, comment intactCHANGELOG.mdsection inserted above## [12.1.0]v12.1.0...v12.1.1, so the tag shape is unchangedactionlintclean on all three workflows.Expected on merge: no Release PR, because
ci:is a hidden type.Review instructions
Merge settings are already correct here, unlike the ruby migration:
allow_merge_commit: false,allow_rebase_merge: false,squash_merge_commit_title: PR_TITLE,squash_merge_commit_message: BLANK. Nothing outside the diff is needed.One consequence worth agreeing on:
masternow has no routine CI. PR runs cover the merge ref, and the release gate covers the commit that gets tagged, but an ordinary merge tomasterruns nothing afterwards.