Support Android API level 27 (STF-772)#42
Conversation
Lower minSdk from 29 (Android 10) to 27 (Android 8.1) so the SDK installs on a wider device base. Two framework calls require API 28 and now route through lossless pre-28 fallbacks on older devices: - InstallationInfoHelper: read the deprecated int versionCode when PackageInfo.longVersionCode is unavailable (< API 28). - DeviceIDsCollector: release MediaDrm via release() instead of close() (the AutoCloseable close() was added in API 28). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin new Robolectric test classes to @config(sdk = [27]) to exercise the pre-API-28 branches added for API 27 support: the legacy int versionCode read in InstallationInfoHelper, and the MediaDrm release()/collect path in DeviceIDsCollector. The JUnit 5 Robolectric extension only allows @config at the class level, so these live in separate classes from the sdk-29/30 tests that cover the modern branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The README listed Kotlin 1.9.22+, but the project builds with Kotlin 2.2.21 (gradle/libs.versions.toml). Update the requirement to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 40 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe minimum supported Android API level is lowered from 29 to 27. ChangesAndroid API 27 compatibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request lowers the minimum supported Android API level from 29 to 27, introducing fallback mechanisms for retrieving the app version code and cleaning up MediaDrm on older API levels. Corresponding Robolectric tests have been added to verify these changes on API 27. The reviewer recommends reverting the Kotlin version update in the README as it is out of scope for this pull request.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - Android API 29+ (Android 10+) | ||
| - Kotlin 1.9.22+ | ||
| - Android API 27+ (Android 8.1+) | ||
| - Kotlin 2.2.21+ |
There was a problem hiding this comment.
This pull request includes an update to the Kotlin version in the README (Kotlin 1.9.22+ to Kotlin 2.2.21+). According to our general rules, we should avoid making out-of-scope edits to pre-existing content that is not the primary focus of the pull request. Please revert this change and address it in a separate PR or commit if necessary.
| - Kotlin 2.2.21+ | |
| - Kotlin 1.9.22+ |
References
- Avoid making out-of-scope edits (such as wording or accessibility improvements) to pre-existing content that is not the primary focus of the pull request.
|
|
||
| - Android API 27+ (Android 8.1+) | ||
| - Kotlin 1.9.22+ | ||
| - Kotlin 2.2.21+ |
There was a problem hiding this comment.
Claude noticed that CLAUDE.md is out of date regarding this.
| */ | ||
| @ExtendWith(RobolectricExtension::class) | ||
| @Config(sdk = [27]) | ||
| internal class InstallationInfoHelperApi27RobolectricTest { |
There was a problem hiding this comment.
Not sure about this, but from Claude:
🟠 MAJOR — the legacy-versionCode test proves nothing (432d75f)
InstallationInfoHelperApi27RobolectricTest.kt:33-39 — the test sets packageInfo.versionCode = 456 and asserts assertEquals(456L, result.versionCode). But in PackageInfo the
deprecated int versionCode and longVersionCode share backing storage: getLongVersionCode() returns (versionCodeMajor << 32) | (versionCode & 0xFFFFFFFF), and versionCodeMajor
defaults to 0 — so longVersionCode also returns 456.
The assertion therefore passes identically whether production takes the >= P (longVersionCode) branch or the else (legacy int) branch. @Config(sdk = [27]) does force the else
branch to execute (good for coverage), but if a regression flipped the if (SDK_INT >= P) condition or deleted the else branch, this test would still go green. That's exactly
the false-confidence case the test was written to prevent. The production code is correct today; the test just can't verify it.
Fix: make the int and long views differ so only the legacy path yields 456 — e.g. packageInfo.setLongVersionCode((7L << 32) or 456L) (or set versionCodeMajor = 7), so
longVersionCode would read (7L<<32)|456 while the legacy int path reads 456L; then assert 456L. Alternatively, downgrade the KDoc's claim to "exercises the else branch
crash-free" rather than "verifies value provenance."
There was a problem hiding this comment.
I don't know if this is possible. Claude updated the comments though.
The test set packageInfo.versionCode = 456 and asserted 456, which a reviewer noted could pass regardless of which branch production took. The suggested fix (pack a non-zero versionCodeMajor into longVersionCode) is not possible here: at @config(sdk = [27]) Robolectric loads the API-27 runtime, where PackageInfo.getLongVersionCode/setLongVersionCode (API 28) do not exist. That same fact is what makes the test discriminate: if the production guard regressed to read longVersionCode at API 27, collect() would throw NoSuchMethodError (it is not caught) and the assertion would never pass. Verified by temporarily flipping the guard. Documented the mechanism in the test instead of relying on value packing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lychee 0.24 changed include_fragments from a boolean to a string enum
(none | anchor-only | text-only | full). After the mise lockfile bumped
lychee to 0.24.2, the old `include_fragments = true` no longer parses
("wanted string or table"), so the Links workflow failed before checking
any links. Use "full" to check both anchor and text fragments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
They are out of date and it would be better for it to check the canoncial source.
oschwald
force-pushed
the
greg/stf-772-support-android-sdk-api-level-27-for-device-tracking
branch
from
be9647e to
fdeec50
Compare
horgh
deleted the
greg/stf-772-support-android-sdk-api-level-27-for-device-tracking
branch
2 participants
Summary
Lowers the SDK's
minSdkfrom 29 (Android 10) to 27 (Android 8.1) so the library installs on a wider device base. Requested in STF-772 — a payment-processor prospect blocked their evaluation on the lack of API-27 support (~7% of their customers on Android 8.1/9).An audit of every Android framework call in
device-sdk/src/mainfound only two call sites that use an API above 27, both API 28, and both have lossless fallbacks — no collected signal is lost on older devices, so nothing needed to be disabled:InstallationInfoHelper— reads the deprecated intPackageInfo.versionCodewhenlongVersionCode(API 28) is unavailable. On API 27 there is noversionCodeMajor, so the value is identical.DeviceIDsCollector— releasesMediaDrmviarelease()instead ofclose()(theAutoCloseableclose()was added in API 28). The DRM ID is read before thefinally, so only cleanup differs.All other framework calls are ≤ API 24 or already guarded (
refreshRate@30,hdrCapabilities@24,getInstallSourceInfo@30).Changes
gradle/libs.versions.toml:minSdk29 → 27 (single source of truth; moves both modules).Build.VERSION.SDK_INT >= Pguards with pre-28 fallbacks.@Config(sdk = [27])since the JUnit 5 Robolectric extension only allows class-level@Config).0.3.0. Also corrected a stale Kotlin version in the README (1.9.22+→2.2.21+), in its own commit.Verification
./gradlew :device-sdk:lintDebug— zeroNewApierrors at minSdk 27 (the authoritative gate that nothing above 27 remains unguarded)../gradlew :device-sdk:test— all pass, including the new API-27 tests../gradlew detekt ktlintCheck— clean../gradlew :device-sdk:assemble— release AAR builds; merged manifest declaresminSdkVersion="27"(no transitive dependency raises the floor).🤖 Generated with Claude Code
Summary by CodeRabbit