Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
|
You can test the changes on this Pull Request by downloading the APKs: |
Sometime when we specify version using a commithash eg. `trunk-1ed207c03d2242b6fc3d74f9e388e9163cbc82a6` a pinned version wins and eg. version `1.22` is used instead of the newer `trunk-1ed207c03d2242b6fc3d74f9e388e9163cbc82a6`.
| implementation "$gradle.ext.wputilsBinaryPath:$wordPressUtilsVersion" | ||
| testImplementation "$gradle.ext.wputilsBinaryPath:$wordPressUtilsVersion" | ||
| debugImplementation "$gradle.ext.wputilsBinaryPath:$wordPressUtilsVersion" | ||
| implementation ("$gradle.ext.wputilsBinaryPath") { |
There was a problem hiding this comment.
All three were identical, so I used only implementation - or am I missing something and we need all three?
I added the strict mode to prevent gradle from wrongly considering a pinned version (eg. 1.22) as more recent than a commit-hash version (eg. trunk-1ed207c03d2242b6fc3d74f9e388e9163cbc82a6). Alternative solution is to publish a new version of Utils.
There was a problem hiding this comment.
Thanks for updating this. It should have been updated while doing the composite build but at the time I was also worried about missing something 😅
| } | ||
|
|
||
| implementation 'com.github.Tenor-Inc:tenor-android-core:0.5.1' | ||
| implementation (name:'tenor-android-core-jetified', ext:'aar') // Jetified Tenor Gif library |
There was a problem hiding this comment.
Tenor hasn't been updated since 2018 - it still uses the support library. I had to jetify the AAR locally and include it in the repo.
| // Debug | ||
| debugImplementation 'com.facebook.stetho:stetho:1.5.1' | ||
| debugImplementation 'com.facebook.stetho:stetho-okhttp3:1.5.1' | ||
| debugImplementation 'com.facebook.stetho:stetho:1.6.0' |
There was a problem hiding this comment.
The only change in the new version is that it uses AndroidX - https://github.com/facebook/stetho/releases
|
|
||
| private void initializePlayer() { | ||
| mPlayer = ExoPlayerFactory.newSimpleInstance(requireContext()); | ||
| mPlayer = (new SimpleExoPlayer.Builder(requireContext())).build(); |
There was a problem hiding this comment.
This was the only breaking change in the new version of ExoPlayer.
| apply plugin: 'com.android.library' | ||
|
|
||
| repositories { | ||
| maven { |
There was a problem hiding this comment.
Adds S3 repository as that's where we store Utils artifacts now.
| @@ -1,5 +1,5 @@ | |||
| buildscript { | |||
| ext.aztecVersion = 'v1.5.1' | |||
| ext.aztecVersion = '946-74d200b819d457d42d405a8858665bd7d471acc9' | |||
There was a problem hiding this comment.
Updates aztec to a version which is using the new version of utils which is not dependant on the support library.
| api ("$gradle.ext.aztecAndroidWordPressCommentsPath:$aztecVersion") | ||
|
|
||
| implementation ("$gradle.ext.wputilsBinaryPath") { | ||
| version { |
There was a problem hiding this comment.
Same as a few comments above: Uses strictly to workaround gradle wrong conflict resolution for pinned vs commithash versions.
# Conflicts: # build.gradle
|
@malinajirka @ParaskP7 I've created a MediaPicker PR that removes the Jetifier as per your suggestions. |
|
👋 @malinajirka !
😅 I actually used I am not sure how you applies the plugin, but I applied it on all modules and gradle configuration files, maybe you just applied it on one module or the top module, not sure how you configured and tested it. 🤔 |
|
👋 @0nko !
This is awesome, thank you! 🙇 |
Fixed by excluding support-annotations from fluxc -> 09ff700. AFAIU it's used only as a hint for static analysis tools -> shouldn't have effect at runtime.
IMO a false positive -> looking at the code of
Fixed by upgrading
Fixed by upgrading I've updated the checklist in the description. |
|
👋 @malinajirka ! Thank you for your continuous efforts on this PR and all these improvements, we are so close into merging it! 🙇
💯
👍 Also, since this is a test related artifact, I am much more relaxed... 😅
🥇 Let's work together in merging this dependant PR! 🚀
🙏
🙇 |
|
FYI: I'm doing the |
ParaskP7
left a comment
There was a problem hiding this comment.
👋 @malinajirka !
Once more, thank you so much for creating this PR, working on it and progressing it till the very end. I reviewed it once more and tested it as per the description. I also applied the bye-bye-jetifier plugin one more, with its new version, the 1.2.2 and verified that we are now jetifier free (except of course org.mockito:mockito-android:3.3.3 which you anyway explained why here).
As such, it is a 🟢 light from my side and we can progress with the remaining merge instructions (step 9 and rest).
PS: I also update the milestone to 19.8 as I would recommend we merge this next Tuesday (19/04/22), right after the 19.7 is cut, just so we get a full 2 weeks of engineering testing before the next beta. Wdyt?
|
Awesome, thank you so much for all the work you've put into this! 🙇
I was planning on suggesting that too 🍻! I'll proceed with the remaining steps and merge on Tuesday/Wednesday. |
|
Found 1 violations: The PR caused the following dependency changes:-+--- org.wordpress:fluxc:{strictly trunk-d49cf4e9d716af757b1c7044a0f786d3015d491e} -> trunk-d49cf4e9d716af757b1c7044a0f786d3015d491e
-| \--- org.wordpress:wellsql:1.7.0
-| \--- com.android.support:support-annotations:28.0.0
-+--- org.wordpress:utils:2.5.0
++--- org.wordpress:utils:{strictly 2.5.0} -> 2.5.0
+--- project :libs:editor:WordPressEditor
-| +--- org.wordpress:aztec:trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 -> v1.5.4
+| +--- org.wordpress:aztec:{strictly trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762} -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762
-| +--- org.wordpress.aztec:wordpress-shortcodes:trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 -> v1.5.4
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.6.10 (*)
-| | +--- org.wordpress:aztec:v1.5.4 (*)
-| | \--- androidx.appcompat:appcompat:1.0.0 -> 1.3.1 (*)
+| +--- org.wordpress.aztec:wordpress-shortcodes:{strictly trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762} -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.6.10 (*)
+| | +--- org.wordpress:aztec:trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
+| | \--- androidx.appcompat:appcompat:1.0.0 -> 1.3.1 (*)
-| +--- org.wordpress.aztec:wordpress-comments:trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 -> v1.5.4
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.6.10 (*)
-| | +--- org.wordpress:aztec:v1.5.4 (*)
-| | +--- androidx.legacy:legacy-support-v4:1.0.0 (*)
-| | \--- com.google.android.material:material:1.0.0 -> 1.6.0-alpha01 (*)
+| +--- org.wordpress.aztec:wordpress-comments:{strictly trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762} -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.6.10 (*)
+| | +--- org.wordpress:aztec:trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
+| | +--- androidx.legacy:legacy-support-v4:1.0.0 (*)
+| | \--- com.google.android.material:material:1.0.0 -> 1.6.0-alpha01 (*)
| \--- org.wordpress-mobile.gutenberg-mobile:react-native-gutenberg-bridge:v1.74.0
| \--- org.wordpress-mobile.gutenberg-mobile:react-native-aztec:v1.74.0
-| +--- org.wordpress:aztec:v1.5.4 (*)
+| +--- org.wordpress:aztec:v1.5.4 -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
-| +--- org.wordpress.aztec:wordpress-shortcodes:v1.5.4 (*)
+| +--- org.wordpress.aztec:wordpress-shortcodes:v1.5.4 -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
-| +--- org.wordpress.aztec:wordpress-comments:v1.5.4 (*)
+| +--- org.wordpress.aztec:wordpress-comments:v1.5.4 -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
| \--- org.wordpress.aztec:glide-loader:v1.5.4
-| \--- org.wordpress:aztec:v1.5.4 (*)
+| \--- org.wordpress:aztec:v1.5.4 -> trunk-f3fc2a67ba38feb4adb4a93703587627aa29c762 (*)
++--- :tenor-android-core-jetified
-+--- com.github.Tenor-Inc:tenor-android-core:0.5.1
-| +--- com.android.support:support-v13:26.1.0
-| | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | \--- com.android.support:support-v4:26.1.0
-| | +--- com.android.support:support-compat:26.1.0
-| | | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | | \--- android.arch.lifecycle:runtime:1.0.0
-| | | +--- android.arch.lifecycle:common:1.0.0
-| | | \--- android.arch.core:common:1.0.0
-| | +--- com.android.support:support-media-compat:26.1.0
-| | | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | | \--- com.android.support:support-compat:26.1.0 (*)
-| | +--- com.android.support:support-core-utils:26.1.0
-| | | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | | \--- com.android.support:support-compat:26.1.0 (*)
-| | +--- com.android.support:support-core-ui:26.1.0
-| | | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | | \--- com.android.support:support-compat:26.1.0 (*)
-| | \--- com.android.support:support-fragment:26.1.0
-| | +--- com.android.support:support-compat:26.1.0 (*)
-| | +--- com.android.support:support-core-ui:26.1.0 (*)
-| | \--- com.android.support:support-core-utils:26.1.0 (*)
-| +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| +--- com.android.support:recyclerview-v7:26.1.0
-| | +--- com.android.support:support-annotations:26.1.0 -> 28.0.0
-| | +--- com.android.support:support-compat:26.1.0 (*)
-| | \--- com.android.support:support-core-ui:26.1.0 (*)
-| +--- com.squareup.retrofit2:converter-gson:2.3.0 (*)
-| \--- com.github.bumptech.glide:glide:3.8.0 -> 4.11.0 (*)
-\--- com.google.android.exoplayer:exoplayer:2.9.3 -> 2.13.3 (*)
+\--- com.google.android.exoplayer:exoplayer:2.13.3 (*)
Please review and act accordingly
|
This is long overdue and shouldn't cause any issues to WPAndroid and/or WCAndroid whatsoever. For more info see WPAndroid PR below, and more specifically the discussion about 'FluxC' and the below final comment on that: Drop jetifier #15675: - wordpress-mobile/WordPress-Android#15675 FluxC was never part of the plan: - https://github.com/wordpress-mobile/WordPress-Android/pull/ 15675#issuecomment-1057840724 ------------------------------------------------------------------------ Also, as part of this change, this 'android.jetifier.ignorelist' Gradle property got removed as well. This Gradle property was added to fix Robolectric tests in CI (df56fb1), then updated to newer equivalent (232ce17), but I am not sure if this is still needed, especially now that Jetifier is disabled.
Merge Instructions:
Review and approve (don't merge) - Update react-native-video from upstream gutenberg-mobile#4361Review this PRMerge Aztec https://github.com/wordpress-mobile/AztecEditor-Android/pull/946/filesMerge Stories Drop jetifier Automattic/stories-android#717Merge GB PR - Update react-native-video from upstream gutenberg-mobile#4361Merge GB PR which updates Aztec Version - it doesn't exist yetUpdate GB hash/version in this repoReview and Merge upgrade of test dependencies - Update espresso and AndroidX test dependencies #16170 (technically not a blocker)Merge the branch in .mobile-secrets into trunkUpdate hash and branch in .configure(9f811ea)Remove "Not ready for merge" labelMerge this PRandroid.enableJetifier=truetoandroid.enableJetifier=falsein their gradle.properties or run./gradlew configureApply.This PR migrates all libraries to versions which are migrated to AndroidX so we can disable jetifier on our builds. Jetifier was always intended to be used only temporarily and is planned to be removed from future versions of AGP.
List of changes
To test:
Make sure to disable Jetifier in your local gradle.properties => Smoke test the app - anything you can think of.
Regression Notes
What I did to test those areas of impact (or what existing automated tests I relied on)
The above "to test" section
What automated tests I added (or what prevented me from doing so)
None - this is a technical improvement which doesn't change the UX
PR submission checklist:
RELEASE-NOTES.txtif necessary.