Skip to content

[Compile Warnings As Errors] WordPress Module - Resolve LifecycleEvent Deprecated Warnings#17223

Merged
ParaskP7 merged 6 commits intotrunkfrom
analysis/wordpress-lifecycle-event-deprecated-warnings
Oct 4, 2022
Merged

[Compile Warnings As Errors] WordPress Module - Resolve LifecycleEvent Deprecated Warnings#17223
ParaskP7 merged 6 commits intotrunkfrom
analysis/wordpress-lifecycle-event-deprecated-warnings

Conversation

@ParaskP7
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 commented Sep 28, 2022

Parent: #17173
Partially Closes: #17181

This PR resolves all LifecycleEvent related Deprecated warnings for the WordPress module:

Classes:

PS: @zwarm I added you as the main reviewer, that is, in addition to @wordpress-mobile/apps-infrastructure team itself, but randomly, since I want someone from the WordPress Android team to primarily sign-off on that change. 🥇

FYI: I am going to randomly add more of you in those PRs that will follow, just so you become more aware of this change and how close we are on enabling allWarningsAsErrors by default everywhere. 🎉

To test:

  • Smoke test both, the WordPress and Jetpack apps, and see if their lifecycle observer related functionalities are working as expected. In more details, and per class, you could test the following (using the debugger or adding extra logs):
    • Init
      • AppInitializer:
        • [onStop]: Put the app in background and verify that the onStop(...) function is called.
        • [onStart]Pull the app from background and verify that the onStart(...) function is called.
      • UploadStarter [onStart]: Pull the app from background and verify that the onStart(...) function is called.
    • Posts
      • PostFetcher [onDestroy]: Enter the Posts screen, then exit the Posts screen and verify that the onDestroy(...) function is called.
      • PostListEventListener [onDestroy]: Enter the Posts screen, then exit the Posts screen and verify that the onDestroy(...) function is called.
    • Stories
      • StoriesEventListener [onDestroy]: Trigger a new Post creation, then go back and verify that the onDestroy(...) function is called.
      • StoryMediaSaveUploadBridge:
        • [onCreate]: Swipe-off the app and kill it, then open the app and verify the onCreate(...) function is called.
        • [onDestroy]: I am not sure how to verify that the onDestroy(...) function is also called, but verifying the onCreate(...) should be enough anyway. PS: From the comment within this function you will notice that the author wasn't sure about that too.

Regression Notes

  1. Potential unintended areas of impact

The lifecycle observer related functionalities are not working as expected, thus causing problems with various background related functionalities of the app (see the To test section above to understand more about that via testing).

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

See To test section above.

  1. What automated tests I added (or what prevented me from doing so)

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.

@ParaskP7
Analysis: Resolve on lifecycle event deprecated warnings for app init
fdc10e6 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7
Analysis: Resolve on lifecycle event deprecated warns for upload starter
a48d274 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7
Analysis: Resolve on lifecycle event deprecated warns for post fetcher
52ffd38 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7
Analysis: Resolve on lifecycle event deprecated warns for post list
a91ac89 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7
Analysis: Resolve on lifecycle event deprecated warns for stories event
7f44f25 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7
Analysis: Resolve on lifecycle event deprecated warns for story media
b60be53 
Warning Message: "'OnLifecycleEvent' is deprecated. Deprecated in Java"

Replacing 'LifecycleObserver' with 'DefaultLifecycleObserver' and then
using this interface's functions via a public override instead of
decorating a function with the '@OnLifecycleEvent' annotation is now the
recommended and more explicit way forward.

PS: The previous annotation based solution required the usage of code
generation or reflection, which should be avoided.

For more info see:
- androidx.lifecycle.OnLifecycleEvent (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
OnLifecycleEvent
- androidx.lifecycle.DefaultLifecycleObserver (Documentation):
https://developer.android.com/reference/androidx/lifecycle/
DefaultLifecycleObserver
@ParaskP7 ParaskP7 added this to the 20.9 milestone Sep 28, 2022
@ParaskP7 ParaskP7 requested review from a team and zwarm September 28, 2022 12:31
@ParaskP7 ParaskP7 self-assigned this Sep 28, 2022
@ParaskP7 ParaskP7 mentioned this pull request Sep 28, 2022
Closed
16 tasks
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Sep 28, 2022

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

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Sep 28, 2022

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

@ParaskP7 ParaskP7 mentioned this pull request Sep 29, 2022
Closed
42 tasks
@AliSoftware AliSoftware modified the milestones: 20.9, 21.0 Oct 3, 2022
zwarm
zwarm approved these changes Oct 3, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ParaskP7 - Looks great and works as expected. I didn't encounter any weirdness while smoke testing the WP and JP apps. Approved, but will leave the merging for you since I'm not sure if just my review will do it.

Nice work 👍

@ParaskP7
Copy link
Copy Markdown
Contributor Author

ParaskP7 commented Oct 4, 2022

Thank you for reviewing and testing this PR @zwarm , you rock! 🙇 ❤️ 🚀

@ParaskP7 ParaskP7 merged commit 9cf8a97 into trunk Oct 4, 2022
@ParaskP7 ParaskP7 deleted the analysis/wordpress-lifecycle-event-deprecated-warnings branch October 4, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zwarm zwarm zwarm approved these changes

Assignees

@ParaskP7 ParaskP7

Labels

Tech Debt Tooling

Projects

None yet

Milestone

21.0

Development

Successfully merging this pull request may close these issues.

Compiler Warnings as Errors - WordPress Module

4 participants

@ParaskP7 @wpmobilebot @zwarm @AliSoftware