Fix #6409: remove Lollipop-era multidex leftovers and freeze Lollipop builds - #6427
Fix #6409: remove Lollipop-era multidex leftovers and freeze Lollipop builds#6427adityabagla7 wants to merge 4 commits into
Conversation
…lipop builds Freeze the final Lollipop Play builds, drop androidx.multidex now that minSdk is 23, clear stale ObsoleteSdkInt suppressions, and narrow the Marshmallow SVG bitmap fallback to API 23 only.
|
@adityabagla7 this PR is being marked as draft because you are not assigned to referenced issue #6409 (or it does not exist). You may only fix issues that you are assigned to to help avoid multiple team members duplicating work on the same issue. |
|
@adityabagla7, you seem to have edited the PR template. Please copy the expected template form here: https://github.com/oppia/oppia-android/blob/develop/.github/PULL_REQUEST_TEMPLATE.md. Please look at other recent PRs on the repo to see how the PR description is expected to look like. |
|
@adhiamboperes, I have updated the PR description to follow the expected template from |
|
Unassigning @adityabagla7 since a re-review was requested. @adityabagla7, please make sure you have addressed all review comments. Thanks! |
|
@adityabagla7 this PR is being marked as draft because you are not assigned to referenced issue #6409 (or it does not exist). You may only fix issues that you are assigned to to help avoid multiple team members duplicating work on the same issue. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe changes freeze legacy release version codes, remove AndroidX Multidex dependencies, delete obsolete lint suppressions, and limit the SVG bitmap fallback to Android API 23. ChangesLollipop cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This cleanup removes obsolete Lollipop multidex support, preserves the Marshmallow SVG workaround, and freezes legacy release codes without an identified regression risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Derive live release version codes in UpdateRolloutFractionTest and UploadChangelogToPlayConsoleTest as 300L so that live release version codes are strictly higher than the frozen baseline 201L, preventing maxOrNull() from selecting the frozen baseline release.
adhiamboperes
left a comment
adhiamboperes
left a comment
There was a problem hiding this comment.
Thanks @adityabagla7! this is a great start.
Can you test the app on an emulator which is running Android 6, and anothe emulator or device with a higher API level, such as 10-16? We want to verify that we don't have unexpected crashes.
What to test:
- opening the app after a fresh install
- play through some lessons, change the app language annd audio language
- open the app after having put it in the background for a few minutes
There was a problem hiding this comment.
Did you edit this file manually or run the script in https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies?
| "alpha" to setOf(16L, 201L), | ||
| "beta" to setOf(196L) |
There was a problem hiding this comment.
The beta version code to be froze will actually be 0.18-rc04-beta-c495933008, which is the current one in prod.
The alpha one is tentatively 0.18-rc04-alpha-c495933008. This might get overwritten tomorrow due to the fact that we are doing weekly alpha releases, so we will update this specifically on the merge date of this PR.
| listOf( | ||
| PlayConsoleClient.TrackRelease( | ||
| versionCodes = listOf(98L, 100L, 99L), status = "inProgress", rolloutFraction = 100 | ||
| versionCodes = listOf(298L, 300L, 299L), status = "inProgress", rolloutFraction = 100 |
There was a problem hiding this comment.
Why have these specific version codes been chosen for the test?
| // --------------------------------------------------------------------------- | ||
| // Deploying to GA (production) — must be less than beta and alpha | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Frozen version codes — ordering constraints must ignore confirmed code | ||
| // (See FrozenReleaseConfig: alpha={16L}) | ||
| // (See FrozenReleaseConfig: alpha={16L, 201L}, beta={196L}) | ||
| // --------------------------------------------------------------------------- |
There was a problem hiding this comment.
So long as we are edding this file, can we get rid of these weird AI generated sectioning? Let there be just one plain single-line comment.
2 participants
Explanation
Fixes #6409
Now that
minSdkVersionis 23, code that only supported Android 5.0/5.1 (API 21/22) is dead and no longer needed:"alpha"to{16L, 201L}and"beta"to{196L}) to prevent them from being silently deactivated on future track updates, and updated the corresponding KDoc list.AbstractOppiaApplicationto extendandroid.app.Applicationinstead ofMultiDexApplicationbecause native multidex is supported on API 21+. Droppedandroidx.multidex:multidexfromapp/BUILD.bazel,third_party/versions.bzl, andthird_party/maven_install.json. Native multidex flavor configurations inbuild_flavors.bzlandinstrumentation/BUILD.bazelremain intact.scripts/assets/maven_dependencies.textprototo remove the definition for the droppedandroidx.multidex:multidexdependency.@SuppressLint("ObsoleteSdkInt")fromAbstractOppiaApplication.onCreate()where no SDK check exists, and fromLocaleController.getSystemLocale()where the guard is>= N(API 24, which is above minSdk 23).SDK_INT <= MtoSDK_INT == MinSvgPictureDrawable.ktto represent the single-version Marshmallow workaround for Fix native picture rendering issue on Marshmallow devices #3961, updated the comment to refer specifically to Marshmallow, and removed the obsolete lint suppression.Essential Checklist
Disclosure of LLM Usage
YesorNo. YesWorkManagerinitialization #6062 for an example.Brainstorming, searching the codebase for Lollipop-related references, verifying build configs, and drafting the PR description.
Summary by CodeRabbit
Bug Fixes
Release Management
Maintenance