Strip build-variant suffix during version normalization (closes #557)#560
Strip build-variant suffix during version normalization (closes #557)#560rainxchzed merged 1 commit intomainfrom
Conversation
…ifier.
Some maintainers ship a single GitHub tag (`1.8.6`) but emit multiple
APK variants whose installed versionName carries a flavour marker
(`1.8.6-f` for full, `1.8.6-m` for minified, `1.8.6-arm64` for
architecture splits). The bare GitHub tag then ranks higher than the
on-device versionName under semver pre-release rules, so the comparator
perpetually surfaces a phantom update.
normalizeVersion now strips a curated set of single-token build-variant
markers (build flavours, distribution channels, architectures) from the
semver pre-release identifier before comparison. Real pre-release
markers (`alpha`, `beta`, `rc`, `preview`, …) and compound
identifiers (`armv7-beta`, `rc.1`) are preserved.
Effects:
- isVersionNewer("1.8.6", "1.8.6-f") → false (was true)
- isVersionNewer("1.8.7", "1.8.6-f") → true (unchanged)
- isVersionNewer("1.8.6", "1.8.6-b") → true (`b` could be beta;
conservative, single-letter `b` not in the strip-list)
- isVersionNewer("1.8.6", "1.8.6-beta") → true (real pre-release,
unchanged)
isExactSameVersion is intentionally unaffected — it still distinguishes
build-metadata variants per its existing docstring (used by the
SmartInstallButton CTA and AppHeader "Install" gate).
Closes #557.
WalkthroughThe change enhances version normalization in ChangesVersion Normalization with Build-Variant Stripping
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt (1)
39-48:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale docstring example after deflavouring is applied.
The
App-v1.2.0-stableexample on line 44 is no longer accurate. After this PR,stableis inBUILD_VARIANT_LITERALSandstripBuildVariantSuffixwill reduce1.2.0-stableto1.2.0. The docstring still advertises1.2.0-stableas the result, which contradicts the very behaviour this PR introduces and is the first place a future reader/maintainer will look.Worth either updating the example to reflect the new output or replacing it with a still-valid case (e.g. an unrecognised single-token suffix that stays intact).
📝 Suggested doc update
- * `App-v1.2.0-stable` → `1.2.0-stable` + * `App-v1.2.0-stable` → `1.2.0` (stable channel marker stripped)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt` around lines 39 - 48, Update the docstring examples in VersionMath.kt to reflect the new deflavouring behavior: change the `App-v1.2.0-stable` example to show `1.2.0` (or replace it with an example that uses an unrecognised single-token suffix if you prefer to demonstrate a preserved suffix). Specifically, edit the examples near the top of the file that describe stripBuildVariantSuffix and BUILD_VARIANT_LITERALS so they match the actual output produced by the stripBuildVariantSuffix function and the BUILD_VARIANT_LITERALS set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt`:
- Around line 39-48: Update the docstring examples in VersionMath.kt to reflect
the new deflavouring behavior: change the `App-v1.2.0-stable` example to show
`1.2.0` (or replace it with an example that uses an unrecognised single-token
suffix if you prefer to demonstrate a preserved suffix). Specifically, edit the
examples near the top of the file that describe stripBuildVariantSuffix and
BUILD_VARIANT_LITERALS so they match the actual output produced by the
stripBuildVariantSuffix function and the BUILD_VARIANT_LITERALS set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55d5157b-feba-43ef-a92a-b6f8d0ae47b5
📒 Files selected for processing (1)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt
What this PR fixes
Closes #557.
Apps that emit multiple APK variants under a single GitHub tag (e.g. tag
1.8.6published, but the APKs install as1.8.6-ffor full /1.8.6-mfor minified /1.8.6-arm64for an architecture split) currently always show as having an update available, because the bare GitHub tag ranks higher than the on-device versionName under semver pre-release ordering rules.VersionMath.normalizeVersionnow recognises and strips a curated set of build-variant / flavour markers from the semver pre-release identifier before comparison. Pre-release markers (alpha,beta,rc,preview,nightly, etc.) are preserved.Algorithm
armv7-beta,rc.1,f-x86_64) are left intact — too risky to disambiguate flavour-vs-pre-release from the tail alone.alpha,beta,rc,preview,prerelease,snapshot,canary,nightly,milestone,ea,dev,pre,m\d+).f,m,l,r,d,xfull,mini,minified,lite,release,debug,extendedstable,final,prod,production,gms,fdroid,github,storearmv7,armv8,arm64,armeabi,x86,x64,x86_64,universal,android,iosBehaviour matrix
isVersionNewer("1.8.6", "1.8.6-f")true(phantom)false✓isVersionNewer("1.8.6", "1.8.6-m")true(phantom)false✓isVersionNewer("1.8.6", "1.8.6-arm64")true(phantom)false✓isVersionNewer("1.8.6", "1.8.6-stable")true(phantom)false✓isVersionNewer("1.8.7", "1.8.6-f")truetrue(real update) ✓isVersionNewer("1.8.6", "1.8.6-b")truetrue(bcould be beta — conservative, untouched)isVersionNewer("1.8.6", "1.8.6-beta")truetrue(real pre-release)isVersionNewer("1.8.6", "1.8.6-rc.1")truetrue(compound, untouched)isVersionNewer("1.8.6-f", "1.8.6-m")false(both strip to1.8.6)Scope
normalizeVersion(and thereforeisVersionNewer,compareVersions,isSameVersion,ExternalInstallVerdict, the periodiccheckForUpdatespath, theDetailsState.findReleaselookup) all benefit from the strip.isExactSameVersionis intentionally unchanged — its existing docstring states it distinguishes build-metadata variants for the install-CTA gate (SmartInstallButton,AppHeader). Preserving that behaviour avoids a surprise change in those callers.Test plan
1.8.6-fversionName, GitHub tag1.8.6). Trigger update check. Verify update badge disappears.1.8.7) → verify update badge re-appears.1.8.6-betastill surface stable1.8.6as available (semver: stable > pre-release).-arm64,-armv7) — phantom update should be gone.Out of scope
assetFilterRegex-equivalent for versionName patterns if the curated list misses anyone.isExactSameVersion. Could be a follow-up for the install button on the details screen, but that's a separate UX call about whether1.8.6-fand1.8.6are "the same release".Summary by CodeRabbit