ci: replace the PR-title bump script with release-please - #76
Conversation
35a0e8b to
8691da5
Compare
mogita
left a comment
There was a problem hiding this comment.
Reviewed at 8691da5. Two Must Fix, four Should Fix, all inline.
The two that matter: src/Client.cs loses its version bump, because no updater covers it once the script is gone, and the manifest seed is one release behind master after v16.0.1 landed today.
Nothing else blocks. The csproj change is whitespace-only with no dependency movement, run_tests.yml is a faithful move of the old ci.yml steps, the detect gating holds on every path I traced, and master has no required status checks, so the job renames break nothing.
Two smaller things left out of the inline comments: the packing SDK moves 9.0.x to 8.0.x (the TFM stays net8.0, but the toolchain producing the shipped nupkg changes), and run_tests.yml requires STREAM_API_KEY as a workflow_call secret while the steps read vars.STREAM_API_KEY || secrets.STREAM_API_KEY. Both exist today, so it works; retiring the secret in favour of the variable would fail the call at start-up.
The bump script derived the version from the merged PR title and pushed the bump, the tag, the GitHub Release and the NuGet package from one job. release-please keeps the bump in a reviewable Release PR instead, and the suite gates the tag. No last-release-sha is needed: this workflow pushed its bump commit to master, so v16.0.0 is reachable and the csproj, the tag and NuGet all agree on 16.0.0. There is one unreleased feat on master, from PR #74, whose release run failed on 2026-09-10, so the first Release PR proposes 16.1.0 and finishes that release. .NET has no first-class release-please strategy, so this uses simple plus an xml extra-file with an xpath at /Project/PropertyGroup/Version. The simple strategy also looks for a version.txt, which does not exist here and is a no-op. The test job moves into a reusable run_tests.yml that both ci.yml and the release gate call, on 8.0.x rather than the release workflow's 9.0.x, matching ci.yml and the net8.0 target. STREAM_API_KEY reads vars first then secrets, because the two workflows disagreed about which held it and both exist. ci.yml drops its push trigger, which ran the suite twice per SHA and raced the live legs against the Stream app several SDK repos share, and pr_title.yml is added to match the other SDKs. src/stream-feed-net.csproj is pre-normalized to the serialization the XML updater emits. BaseXml reparses and rewrites the whole document, so without this the first Release PR would carry unrelated whitespace churn on four PackageReference lines alongside the version bump.
src/Client.cs carries a second version literal that the deleted bump script kept in step with the csproj. release-type simple adds no source updater, so nothing would have bumped it: VersionName feeds the X-Stream-Client header and the stream.sdk.version log field, and both would have frozen at 16.0.1 while the package moved on. Covered by an extra-file with the usual marker comment. Merging #77 fired the old release workflow, which cut v16.0.1 and shipped #74's feat with it, so the seed and the changelog move to 16.0.1 and the changelog header no longer promises 16.1.0. The NuGet push took a glob with --skip-duplicate, which exits 0 and publishes nothing when the packed version does not match the tag. It now pushes the exact path and fails when that file is absent, and re-attaches the nupkg to the GitHub Release, which every release before this one carried and release-please does not. run_tests.yml gains a Release-configuration test leg. The package is packed from Release and the old release.yml was the only place that tested it. ci.yml covers PRs into N.x, and STREAM_API_KEY drops to required: false because the steps fall back to the repository variable.
8691da5 to
e043484
Compare
|
Review addressed in e043484. Both must-fix and all four should-fix are fixed, replies on each thread. Correcting the description, which is now wrong in the way the manifest comment describes: merging #77 fired the old On the two smaller notes: Packing SDK 9.0.x to 8.0.x. Deliberate, and worth a second opinion. The TFM is
|
Adding a second Release leg alongside Debug ran the whole live suite twice per job. HardDeleteChannels then failed four times in a row, once in each leg, on an async hard-delete task that gets 30 one-second polls; the same test passed twice at 11:57 and 12:03 today, when a job ran the suite once. Release is the configuration the package is packed from, so running only that one answers the original point without doubling the async-task load on an app that getstream-php shares.
Ticket
CHA-2963. Fifth repo, after getstream-go, stream-py (#287), getstream-ruby (#86) and getstream-php (#65).
Problem
The bump script derived the version from the merged PR title and then tagged, released and pushed to NuGet from a single job. One unreleased
feat:is sitting onmasterright now because that job failed on 2026-09-10.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, manifest seeded at16.0.0. Nolast-release-sha: this workflow pushed its bump commit tomaster, sov16.0.0is reachable and the csproj, the tag and NuGet all agree.simpleplus anxmlextra-file at/Project/PropertyGroup/Version. Thesimplestrategy also looks for aversion.txt, which does not exist here and is a no-op.src/stream-feed-net.csprojis pre-normalized to the serialization the XML updater emits.BaseXmlreparses and rewrites the whole document, so without this the first Release PR would carry whitespace churn on fourPackageReferencelines next to the version bump.dotnet restorepasses on the normalized file.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, on8.0.xrather than the release workflow's9.0.x, matchingci.ymland thenet8.0target.STREAM_API_KEYreadsvarsthensecrets, because the two workflows disagreed about which held it and both exist.ci.ymldrops its push trigger, which ran the suite twice per SHA and raced the live legs against the Stream app several SDK repos share.pr_title.ymlis added to match the other SDKs.dotnet nuget push --skip-duplicate, andworkflow_dispatchtakespublish_tagso an existing tag can be pushed after GitHub retires the original run.scripts/bump_version.sh, and backfills the changelog for 15.1.0 through 16.0.0.How to verify
Dry-run against a throwaway branch carrying one
fix:commit (release-please release-pr --dry-run --trace), now deleted:16.1.0from basev16.0.0, which is the unreleasedfeat:from feat: [FEEDS-1763.1] regenerate OpenAPI client #74 finally shippingsrc/stream-feed-net.csproj<Version>16.0.0->16.1.0, and nothing else in the file movesCHANGELOG.mdsection inserted above## [16.0.0]v16.0.0...v16.1.0, so the tag shape is unchangedactionlintclean on all four workflows.dotnet restoreclean on the normalized csproj.Expected on merge: a Release PR for 16.1.0, unlike the other four migrations.
ci:is a hidden type, but #74'sfeat:is still unreleased onmaster, so there is a real release waiting.Review instructions
The first Release PR's changelog will list
[FEEDS-1763.1] regenerate OpenAPI clienttwice, once for the merge commit and once for the branch commit under it. That is history from before the repo went squash-only and it cannot be rewritten; later releases will not do it.Merge settings are already correct:
allow_merge_commit: false,allow_rebase_merge: false,squash_merge_commit_title: PR_TITLE,squash_merge_commit_message: BLANK.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.