-
Notifications
You must be signed in to change notification settings - Fork 22
Update to Android SDK 35. #236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hi @rtibbles the app is not working on both my Android 7.1.1 and Android 8 phones, and also in an Android Studio emulator running Android 7 (the lowest version I could install): 2025-09-08_14-49-24.mp4Here are the logcat logs from the emulator as there are no logs in the The app is working correctly on my Android 10 phone and in an Android Studio emulator running Android 16. |
|
Yes, it seems like testing on Android 6 is nigh on impossible now, so I should probably increase the minimum version to 7. |
Cleanup easily fixed gradle deprecation warnings.
1ba87a9 to
db94570
Compare
|
Hi @rtibbles - I confirm that now the app is working correctly on both my Android 7 and 16 emulated devices as well as on my other physical devices. |
marcellamaki
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have read through the changes and cross referenced them with documentation about dependences as well as the FrameLayout. Although I do not feel especially confident in "this is the best way to build this" it does seem more or less aligned with what i'm seeing in the documentation.
one small nitpick change (non-blocking) for the documentation reference link (even though I was able to find the relevant page without much trouble just by going to the home page and searching 🤷♀️ )
| buildConfig = true | ||
| } | ||
| androidResources { | ||
| noCompress 'tflite' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any sense of why the syntax changes, and why the androidResources is the exception with no =? Although I guess scrolling up to look at sourceSets that also didn't change and the android thing is that it's been renamed (and moved to '')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I just did what Android Studio told me to.
Summary
Updates target SDK to 35
References
Fixes #235
Reviewer guidance
Smoke test asset, does it work?
Of particular interest is making sure that this works on older Android devices (I tried to retain compatibility back to Android 6).
For any code reviewers, please be aware that Claude was largely responsible for the final commit to deal with the layout on edge to edge compliant Android versions (15+). I cross checked that the solutions were broadly similar to the documentation and linked blog posts on the Android developers site - but mostly, I was satisfied that it just worked when I tested it.