-
Notifications
You must be signed in to change notification settings - Fork 450
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
Gradle, Build scripts, Maven Plugin, NDK update. #229
Conversation
NDK update 16KB page size support
libraries updated
Properties check from the root project in gradle Unnecessary null check removed
This looks great. I am away ATM but will review this upon return next week. We really value the effort here. 👌 |
app/build.gradle.kts
Outdated
applicationId = "com.scottyab.rootbeer.sample" | ||
minSdk = libs.versions.android.min.sdk.get().toInt() | ||
targetSdk = libs.versions.android.target.sdk.get().toInt() | ||
versionName = "0.9" |
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.
We're just missing versionCode = 11
otherwise the generated .apk is RootBeerSample-0.9-[null]-release.apk
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.
Fixed both versionName and versionCode, now they are specified using properties. As far as I understood it was the initial intention, please correct me if I'm wrong.
It all pretty much checks out ✅ , again huge thanks! this saved us a bunch of time and sets us up to update the library. Last remaining task for me is to confirm the Maven publishing to Nexus works with the updated plugin (and I still remember how to do it 😉 ). Then we can merge and I'll look at the other PRs (long overdue) and publish an updated version of the lib. The Sample app will take a bit more effort due to new Playstore requirments but this can be a seperate task. |
Thank you very much for review and the lib itself) |
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.
Confirmed Nexus publishing works as expected after I switched to using tokens (not a change that's needed to the code here). Will merge later. Thanks again!
Rebasing the merge rather than the usual squash merge due to the amount of changes here. |
List of updates: