tools: Add version computation and native SDK pinning to releaser. - #1130
tools: Add version computation and native SDK pinning to releaser.#1130fuzzybinary wants to merge 11 commits into
Conversation
…leaser. Computes package version bumps from conventional commits (including BREAKING/refs footers) and resolves iOS/Android/C++ native SDK pins er trigger context (mainline/patch/pre-release), wiring both into release_plan.dart's ReleasePlan output. Consolidates duplicated regex/parsing logic that previously lived separately in cocoapod_util.dart, gradle_util.dart, and generate_changelog.dart into shared native_sdk.dart and conventional_commits.dart utilities, and splits pure git-history queries (findLastReleaseTag, commitMessagesSince) into their own git_history.dart module. refs: RUM-17812 RUM-18022 # Conflicts: # tools/releaser/test/package_discovery_test.dart # tools/releaser/test/release_plan_test.dart # tools/releaser/test/support/fixture_repo.dart
fuzzybinary
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c6a613e29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Refactor version pinning information to eliminate special cases and instead have the "source" of the pin and the version that was pinned. This is so the iOS SDK and the C++ SDK can report what versions are pinned in SPM / Cocoapods or Windows / Linux respectively without special cases for each, and allows us to look at pinned version for both Linux and Windows (linux was previously being ignored.) Fix issue where we would get the wrong last release when patching older releases.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13f1f31e88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
…on-native-sdk-pinning
findLastReleaseTag now requires the chosen tag to be an ancestor of HEAD, so a tag cut on a line that never merged back (e.g. a long-lived prerelease branch) can't be mistaken for the real last release just because it sorts higher by version number. pinCppVersion now only rewrites a GIT_TAG line while inside the dd-sdk-cpp FetchContent_Declare block, so a second, unrelated FetchContent_Declare in the same CMakeLists.txt keeps its own GIT_TAG.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cf4ae021d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ck scoping - Compare mainline's native SDK pins against what was actually pinned at the last release tag (via git show), instead of develop's own intentionally-floating file, which made every mainline run report a spurious native SDK change. - Scope readCppCMakePin to the dd-sdk-cpp FetchContent_Declare block, sharing the paren-balance tracking with cmake_util.dart's pinCppVersion so a CMakeLists.txt with another FetchContent_Declare above dd-sdk-cpp's isn't misread. - Dedupe the patch/preRelease _computeNativeSdkDeltas call and log instead of silently dropping a pin when its file can't be read at the last release tag.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cef8c2aa3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…release-list paging bugs - Mainline now promotes a merged prerelease tag to the stable version it was leading up to, instead of bumping past it (e.g. 4.0.0-beta.5 -> 4.0.1, skipping the intended 4.0.0 release). - A native dependency manifest with no historical pin to compare against (added or renamed since the last release) now counts as a native SDK change instead of being silently dropped from consideration. - Starting a new pre-release for a target version that's already been released stably now fails loudly instead of producing a version that sorts below the published release. - gh release list now passes --limit so an explicit SDK version override against an older release isn't rejected just for falling past the CLI's default page of 30.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f805e51f98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion
The native SDK check grew a current-pin-vs-target comparison that isn't in
the design doc, where step 1c only *determines* the target version. That
comparison is unanswerable on `develop` by construction -- develop keeps its
manifests on floating constraints by design and only the release-prep branch
is ever pinned -- so each review round patched around it with another
concept: per-file pins, then reading the pin out of the last release tag's
blob, then hasUnknownPin for when that blob is missing, then again for when
it exists without a pin. Drop the comparison instead.
- NativeSdkDelta is now {sdk, targetVersion, targetSha, files}: a target plus
the files to rewrite. Deletes pins/NativeSdkPin, hasUnknownPin, isChange,
spmPinForComparison, the four read*Pin helpers, and the historical-blob
read. The shared regexes stay -- discovery and the pin-writing utils still
share one definition of each line shape.
- Eligibility follows the design doc: qualifying commits, explicitly
requested, or an explicit native SDK override for an SDK the package
actually ships a manifest for (so IOS_SDK_VERSION can't sweep pure-Dart
packages into an --all run). Decided before any network call, which also
means the three plan functions are synchronous and non-nullable.
findLastReleaseTag no longer filters by reachability from HEAD. Release tags
land on `release/...` branches that are never merged back, so no release tag
is an ancestor of `develop` or of a pre-release branch like `v4` -- the
ancestor check rejected every real release and fell back to ancient
prerelease tags (datadog_flutter_plugin computed 3.6.0 -> 2.0.0, a
downgrade, from a 140-commit range). The concern it addressed -- a
4.0.0-beta.3 tag off v4 outranking mainline's last stable -- is handled by
version shape instead: mainline takes the highest stable tag, patch is
constrained by releaseLine, pre-release wants those betas and constrains
neither. That also makes the prerelease-promotion branch unreachable, since
mainline can no longer select a prerelease tag; the natural path (last
stable + aggregated bump) reaches the same 4.0.0.
Also fixed, all found while removing the above:
- CMakeLists discovery and rewriting disagreed about scope -- discovery
matched any GIT_TAG in windows/linux CMakeLists.txt while pinCppVersion was
scoped to the dd-sdk-cpp FetchContent_Declare block. Both now share one
block scanner.
- getReleaseByTagName scanned a paged `gh release list` (30 by default, so a
valid older override could be rejected for falling off the page). Replaced
with releaseExists hitting repos/{slug}/releases/tags/{tag} directly.
- release_validator caught a bare StateError and reported "Could not find
target release" for it, mislabeling any unrelated failure. It now checks
releaseExists directly rather than routing through a throwing helper.
- _computePrereleasePlan left contributingCommits empty, which would have
given the changelog step no input on a pre-release ship.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea37d0af84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tchContent An unconstrained lookup returns the package's highest tag overall, so a pre-release run targeting 4.0.0 picks up an unrelated `4.0.1` patch or a concurrent `5.0.0-beta.1`. Neither is on the target line, so the plan falls back to the pubspec version as its base and regenerates `4.0.0-beta.1` -- a tag that already exists, which fails on push. Fix by scoping the release line to one of three options: mainline -- the last stable release patch -- the last release on the branch's release line pre-release -- the last release at the exact declared target version The scope comes from the pubspec version, so nothing new has to be passed in. `releaseLine` generalizes to `versionScope` with a nullable patch component -- patch runs leave it open, pre-release pins it, since "the last release at 4.0.0" and "the last release on the 4.0 line" are different questions. Additionally, `FetchContent_Declare(dd-sdk-cpp ... GIT_TAG develop)` written on one line is valid CMake, but the GIT_TAG pattern was anchored to the start of a line, so neither discovery nor rewriting could see it. Still not handled: a declaration split as `GIT_TAG\n develop` across two lines. transformFile is line-by-line, so that needs a different approach, and no manifest in this repo is written that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b383158735
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Reject overriding the bump_type without an explicit packages list - Be more tolerant of spacing differences in native SDK files - Prevent backwords moves of pre-releases.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a8c45bf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| final commits = await _conventionalCommitsSince( | ||
| gitDir, | ||
| pathspec: pkg.relativePath, | ||
| sinceSha: lastTag?.tag.objectSha, |
There was a problem hiding this comment.
Fresh evidence after the earlier prerelease --all fix is that the exact-target tag used for the prerelease counter is also reused as the commit-range baseline. When a package has an older stable release but no tag yet at its post-release pubspec target, lastTag is null and commitMessagesSince scans the package's entire history; any already-shipped historical feat: or fix: then qualifies an otherwise untouched package and assigns it a new -beta.1. Use the latest prior release tag when collecting commits and deciding eligibility, while retaining the exact-target lookup separately for continuing the prerelease counter.
Useful? React with 👍 / 👎.
|
Rethinking this as the number of edge cases has gotten unreasonable. We're going to take a different approach to figuring out the "last release" and the required version bump type. |
1 participant
What and why?
Computes package version bumps from conventional commits (including BREAKING/refs footers) and resolves iOS/Android/C++ native SDK pins er trigger context (mainline/patch/pre-release), wiring both into release_plan.dart's ReleasePlan output.
Consolidates duplicated regex/parsing logic that previously lived separately in cocoapod_util.dart, gradle_util.dart, and generate_changelog.dart into shared native_sdk.dart and conventional_commits.dart utilities, and splits pure git-history queries (findLastReleaseTag, commitMessagesSince) into their own git_history.dart module.
Two things look like omissions but are deliberate — both were "fixed" during review and reverted:
Native SDK deltas are targets, not diffs. Nothing compares a manifest's current pin against the resolved target.
developpurposefully keeps its native pins floating by design and only the release-prep branch is ever pinned, so there is no current pin to diff — any comparison has to reach into history and gets steadily more special-cased. A package is eligible when it has qualifying commits, was explicitly requested, or has an explicit IOS_SDK_VERSION/ANDROID_SDK_VERSION/CPP_VERSION override for an SDK it actually ships a manifest for. This was the original design.findLastReleaseTag does not filter by reachability from HEAD. Release tags land on
release/…branches and are never merged back, so no release tag is an ancestor of develop. An ancestor check rejects every real release and falls back to ancient prerelease tags. The concern was that prerelease tag 4.0.0-beta.3 tag off v4 would outrank the last stable release on mainline is handled by version shape instead: mainline takes the highest stable tag (stableOnly), patch is constrained by releaseLine, pre-release wants those betas and constrains neither.refs: RUM-17812 RUM-18022
Review checklist