-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
chore(android): gradle 8 #14014
chore(android): gradle 8 #14014
Conversation
chore(android): fixed deprecations in kroll-apt project chore(android): made kroll-apt incremental to enhance build time
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.
Are the changes to the JAVA files necessary for the update? The more stuff that is changed, the more we have to test 😉
Also I wouldn't update the min and targetSDK with this PR. If we still can use 21 we should do that too. Same with target 34. I have a PR for that here #13940 as the broadcast receiver has to be adjusted.
So if that is not needed for the gradle PR please keep it as it is and do it in a separate PR (less changes, less testing).
I couldn't build a SDK locally. It's almost working but the last step titaniumPublication(MavenPublication)
tries to publish it online instead of making a local repo? I've canceled it as it was uploading stuff 😄 Didn't want to publish something I don't need.
There aren't all cmake version available during build process. I have a different PR about that #13966 and I had to use 3.22.1
as that is available on all platforms.
fix(android): gradle 8 compatibility update to template build.gradle fix(android): revert dependency updates fix(android): revert minSdk and targetSdk
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.
All reviews applied.
fix(android): update _build.js & _buildModule.js for new gradle
I have to do some testing but the first round was very good and I was able to build kitchensink an my app now 👍 Review will follow once I have more time to test |
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.
This is AWESOME! I don't wanna lose momentum on this PR.
I found a few indention nitpicks. I haven't tested the PR yet, but I spent some time researching some of the changes such as no longer need to call .setPackageName()
. Good stuff!
What sort of testing do we need to do to get this over the finish line? I can test on Windows doing emulator builds as well as build-only device/dist builds. I don't have any working Android devices. Sigh.
👍 @cb1kenobi I would love to split this up into smaller steps as this is a big PR: |
Co-authored-by: Chris Barber <[email protected]>
Co-authored-by: Chris Barber <[email protected]>
Co-authored-by: Chris Barber <[email protected]>
Co-authored-by: Chris Barber <[email protected]>
Co-authored-by: Chris Barber <[email protected]>
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.
A few questions and changes we should verify. And can you please rebase with latest master to resolve the merge conflicts?
Co-authored-by: Chris Barber <[email protected]>
@m1ga |
Of course |
Did a first quick test. EDIT: I've pushed the material.R changes! SDK will build now 👍 After that: SDK builds fine and I tested it with two apps with existing modules and they work fine. Kitchensink works too. Hyperloop-examples breaks:
same when building modules:
|
@m1ga PR's can be checked out most easily with the Github CLI and |
@hansemannn nice! That worked 👍 SDK will build now, modules & hyperloop currently fail with the error above. @AbdullahFaqeir would be nice if you can find out where the namespace is missing. Apps without hyperloop work fine but I didn't check all controls yet. |
android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIButton.java
Outdated
Show resolved
Hide resolved
@m1ga @hansemannn I fixed it to build for modules, couldn't build tho on my Silicon machine, but it the fix for sure went after that stage. |
awesome! SDK builds, apps build, modules build 👍 We are getting closer.
|
one thing we'll need to tell the module developers: minSdk 12.6.0 is required when you build modules with this SDK as it is building with a higher Java version. The other way (old modules in 12.6.0 apps) works fine. |
@m1ga while generating the manifest file? |
when building an app with a module that was build with this PR you'll see |
@m1ga Exactly! |
@m1ga @hansemannn I believe the PR is ready to be merged? |
@AbdullahFaqeir I think we should add the new hyperloop module to this PR too as it is needed for it to work. For that tidev/hyperloop.next#391 should build the Github repo so we have a release version. If it is released we can change the version number in: titanium-sdk/support/module/packaged/modules.json Lines 57 to 58 in fac2c4b
so it will be delivered with the SDK |
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.
My tests look good:
- building the SDK ✔️
- building app with multiple existing modules with this SDK ✔️
- building modules and the included example app ✔️
- building hyperloop-examples ✔️
- building kitchensink-v2 ✔️
android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIButton.java
Outdated
Show resolved
Hide resolved
Tested on Windows 11, Titanium CLI 7.1.4, Node.js 22.6.0, Java 20.0.1 and my dead simple app (no modules) works great! |
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.
LGTM!
LET'S GOOOO 🚀🚀🚀 |
Excellent work! |
Halla walla, That's Awesome @AbdullahFaqeir you saved my day |
chore(android): let there be gradle 8
chore(android): fixed deprecations in kroll-apt project
chore(android): made kroll-apt incremental to enhance build time
GitHub issue: #13579
This PR is dedicated to my mentor Nafe Abboushi.