Skip to content
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

Parent PR for updating compileSdk 33 #17947

Merged
merged 60 commits into from
Mar 22, 2023
Merged

Conversation

irfano
Copy link
Member

@irfano irfano commented Feb 12, 2023

Fixes #17790

This PR,

  • Creates a parent branch (feature/update-compile-sdk-33),
  • Disables all warnings as errors (those warnings will be resolved in separate PRs),
  • Resolves errors to make the PR compilable.

Next required tasks that will be merged into feature/update-compile-sdk-33 via separate PRs:

Warning for reviewers
This PR shouldn't be merged unless all the required tasks above are completed.


To test:

  • Thoroughly smoke-test both the WordPress and Jetpack apps and see if they both work as expected.

Regression Notes

  1. Potential unintended areas of impact
    As this affects a lot of classes, every screen may be impacted.
  • The back navigation should work as it did before, without any crashes,
  • Navigation between screens should work without crashing or any data loss,
  • Deep links,
  • Jetpack migration.
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
    I smoke-tested notifications and screen navigations.

  2. What automated tests I added (or what prevented me from doing so)
    None. This doesn't add any new functionality to the app.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Errors were disabled temporarily to be able to fix all errors in separate PRS. It'll be re-enabled after all errors have been fixed.
These functions were changed with Android 13.
These functions were changed with Android 13.
These functions were changed with Android 13.
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 12, 2023

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17947-3143cc1.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commit3143cc1
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 12, 2023

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17947-3143cc1.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commit3143cc1
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@irfano irfano self-assigned this Feb 12, 2023
@ParaskP7 ParaskP7 self-assigned this Feb 13, 2023
@ParaskP7
Copy link
Contributor

👋 @irfano !

FYI: I just reviewed the first 8 commits which effectively create this parent feature/update-compile-sdk-33 branch. Everything seems good to me, so it is a 👍 from my side on those.

PS: I also quickly smoke tested both, the WordPress and Jetpack apps, and the basics of functionality like posts/pages/stories, or navigating through the basic screens like my site/reader/notifications, and everything seems to be working as expected.

@irfano irfano added this to the 22.1 milestone Feb 25, 2023
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 15, 2023

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr17947-4e3568d
Commit4e3568d
Direct Downloadwordpress-prototype-build-pr17947-4e3568d.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 15, 2023

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr17947-4e3568d
Commit4e3568d
Direct Downloadjetpack-prototype-build-pr17947-4e3568d.apk
Note: Google Login is not supported on these builds.

@pachlava
Copy link
Contributor

@irfano 👋

I'm still testing, but wanted to communicate earlier something that looks like a crash, and it's not reproducible with 21.9-rc-2. It happens when I try to select a category for a new story post:

  1. Connect to a site that supports creation of story posts
  2. Start creating a story post
  3. Select an image and continue
  4. Tap Categories
az_recorder_20230316_151551.mp4

I reproduced it on Samsung A31 with Android 12 and the latest JP installable from this PR.

It's not reproducible with Pixel 7 + Android 13, and also not reproducible with JP 21.9-rc-2 on both devices.

I'll come back soon with info if it's rep on Emulator, so there might be some useful logs (I don't know how to retrieve the logs from the device after the app crash).

@pachlava
Copy link
Contributor

I could reproduce the case above with Pixel 2 API 31 Emulator:

2023-03-16 15:43:09.128 4224-4224/com.jetpack.android.prealpha D/AndroidRuntime: Shutting down VM
    
    
    --------- beginning of crash
2023-03-16 15:43:09.133 4224-4224/com.jetpack.android.prealpha E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.jetpack.android.prealpha, PID: 4224
    java.lang.NullPointerException: null cannot be cast to non-null type org.wordpress.android.ui.posts.PrepublishingAddCategoryRequest
        at org.wordpress.android.ui.posts.PrepublishingCategoriesFragment.initViewModel(PrepublishingCategoriesFragment.kt:242)
        at org.wordpress.android.ui.posts.PrepublishingCategoriesFragment.onViewCreated(PrepublishingCategoriesFragment.kt:88)
        at androidx.fragment.app.Fragment.performViewCreated(Fragment.java:3128)
        at androidx.fragment.app.FragmentStateManager.createView(FragmentStateManager.java:552)
        at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:261)
        at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1899)
        at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1817)
        at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1760)
        at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:547)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7839)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

@irfano
Copy link
Member Author

irfano commented Mar 16, 2023

Great job, @pachlava! Good catch! It probably caused crashes in other areas too.
I fixed the crash. Can you test it on the new build?

# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/JetpackRemoteInstallActivity.kt
#	WordPress/src/main/java/org/wordpress/android/ui/JetpackRemoteInstallFragment.kt
#	WordPress/src/main/java/org/wordpress/android/ui/jpfullplugininstall/install/JetpackFullPluginInstallActivity.kt
@pachlava
Copy link
Contributor

pachlava commented Mar 17, 2023

Hey @irfano!

Thank you for the fix 🙇 The installable from d49b8d7 does not crash 👍

I did not notice anything else, apart from two things that are not caused by this PR:

  • Google login is not working, but this is expected for Jalapeno Debug builds.
  • Back navigation is not working in several Blaze-related screen, but this is also true for 21.9-rc-2, so I'm going to communicate that separately.

Thank you!

UPD: just found out that the second point is on purpose (p5T066-3Ue-p2#comment-14484)

@irfano
Copy link
Member Author

irfano commented Mar 17, 2023

I made some changes after @pachlava's testing, which became necessary after syncing with the trunk. @ParaskP7, could you review the latest commits that were pushed after the last sub-PR was merged?

@ParaskP7
Copy link
Contributor

I made some changes after @pachlava's testing, which became necessary after syncing with the trunk. @ParaskP7, could you review the latest commits that were pushed after the last sub-PR was merged?

I am on-it @irfano , thanks for the ping! 👍 👀

@ParaskP7
Copy link
Contributor

I am on-it @irfano , thanks for the ping! 👍 👀

Done @irfano , thanks for the update, the changes look good (✅). However, I didn't test the JetpackRemoteInstallActivity related screen, maybe it is worth-it to test it (again), just to be sure that we are not missing something. Cc @pachlava

@pachlava
Copy link
Contributor

I didn't test the JetpackRemoteInstallActivity related screen, maybe it is worth-it to test it (again), just to be sure that we are not missing something. Cc @pachlava

I didn't go for a thorough test this time, but JP plugin remote installation worked in the latest installable for me, I checked on both Android 12 and 13.

@irfano
Copy link
Member Author

irfano commented Mar 21, 2023

Thanks for testing @pachlava!
@ParaskP7, can we merge this now?

@ParaskP7
Copy link
Contributor

@ParaskP7, can we merge this now?

Yes, I believe we can @irfano , feel free to merge this when you're ready! 🚀

@irfano irfano requested a review from ParaskP7 March 22, 2023 12:50
@irfano irfano merged commit 701b8f4 into trunk Mar 22, 2023
@irfano irfano deleted the feature/update-compile-sdk-33 branch March 22, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update "compileSdk" to 33
4 participants