[Dependency Updates] Update glideVersion to 4.15.1#18557
Conversation
Release Notes: https://github.com/bumptech/glide/releases/tag/v4.10.0 ------------------------------------------------------------------------ Glide v4 Volley (Docs): https://bumptech.github.io/glide/int/volley.html ------------------------------------------------------------------------ This change effectively updates Glide's 'volley-integration' from '4.6.1@aar' to '4.10.0'. FYI: This change also drops the '@aar' suffix requirement, as the per the 'Glide v4 Volley' documentation. PS: The only difference between having the '@aar' suffix and not having it, is the fact that not having it also bring in transitive dependencies related to that 'volley-integration' dependency.
|
Found 1 violations: The PR caused the following dependency changes: +--- project :libs:editor
| \--- org.wordpress-mobile.gutenberg-mobile:react-native-gutenberg-bridge:v1.96.0
| +--- org.wordpress-mobile.react-native-libraries.v1:react-native-fast-image:8.5.11
-| | +--- com.github.bumptech.glide:glide:4.12.0
-| | | +--- com.github.bumptech.glide:gifdecoder:4.12.0
-| | | | \--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
-| | | +--- com.github.bumptech.glide:disklrucache:4.12.0
-| | | +--- com.github.bumptech.glide:annotations:4.12.0
-| | | +--- androidx.fragment:fragment:1.0.0 -> 1.5.7 (*)
-| | | +--- androidx.vectordrawable:vectordrawable-animated:1.0.0 -> 1.1.0 (*)
-| | | \--- androidx.exifinterface:exifinterface:1.2.0 -> 1.3.6 (*)
+| | +--- com.github.bumptech.glide:glide:4.12.0 -> 4.15.1
+| | | +--- com.github.bumptech.glide:gifdecoder:4.15.1
+| | | | \--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
+| | | +--- com.github.bumptech.glide:disklrucache:4.15.1
+| | | +--- com.github.bumptech.glide:annotations:4.15.1
+| | | +--- androidx.fragment:fragment:1.3.6 -> 1.5.7 (*)
+| | | +--- androidx.vectordrawable:vectordrawable-animated:1.1.0 (*)
+| | | +--- androidx.exifinterface:exifinterface:1.3.3 -> 1.3.6 (*)
+| | | \--- androidx.tracing:tracing:1.0.0 (*)
| | \--- com.github.bumptech.glide:okhttp3-integration:4.12.0
-| | \--- com.github.bumptech.glide:glide:4.12.0 (*)
+| | \--- com.github.bumptech.glide:glide:4.12.0 -> 4.15.1 (*)
| \--- org.wordpress-mobile.gutenberg-mobile:react-native-aztec:v1.96.0
| \--- org.wordpress.aztec:glide-loader:v1.6.3
-| \--- com.github.bumptech.glide:glide:4.10.0 -> 4.12.0 (*)
+| \--- com.github.bumptech.glide:glide:4.10.0 -> 4.15.1 (*)
+--- com.automattic:stories:2.1.0
-| +--- com.github.bumptech.glide:glide:4.10.0 -> 4.12.0 (*)
+| +--- com.github.bumptech.glide:glide:4.10.0 -> 4.15.1 (*)
| +--- jp.wasabeef:glide-transformations:4.3.0
-| | \--- com.github.bumptech.glide:glide:4.11.0 -> 4.12.0 (*)
+| | \--- com.github.bumptech.glide:glide:4.11.0 -> 4.15.1 (*)
| \--- com.automattic.stories:photoeditor:2.1.0
-| +--- com.github.bumptech.glide:glide:4.10.0 -> 4.12.0 (*)
+| +--- com.github.bumptech.glide:glide:4.10.0 -> 4.15.1 (*)
| \--- com.automattic.stories:mp4compose:2.1.0
-| \--- com.github.bumptech.glide:glide:4.10.0 -> 4.12.0 (*)
+| \--- com.github.bumptech.glide:glide:4.10.0 -> 4.15.1 (*)
-+--- com.github.bumptech.glide:glide:4.10.0 -> 4.12.0 (*)
++--- com.github.bumptech.glide:glide:4.15.1 (*)
-\--- com.github.bumptech.glide:volley-integration:4.6.1
+\--- com.github.bumptech.glide:volley-integration:4.15.1
+ +--- com.github.bumptech.glide:glide:4.15.1 (*)
+ +--- com.android.volley:volley:1.2.0 -> 1.2.1
+ \--- androidx.annotation:annotation:1.3.0 -> 1.6.0 (*)
Please review and act accordingly
|
|
 |
App Name |  WordPress |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18557-0e8c74c | |
| Commit | 0e8c74c | |
| Direct Download | wordpress-prototype-build-pr18557-0e8c74c.apk |
|
 |
App Name |  Jetpack |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18557-0e8c74c | |
| Commit | 0e8c74c | |
| Direct Download | jetpack-prototype-build-pr18557-0e8c74c.apk |
irfano
left a comment
There was a problem hiding this comment.
👏🏻 Great job on comprehensive testing instructions. I couldn't find any changes in the Glide changelogs that could break the app.
5b. Activity Log Detail Screen [ActivityLogDetailFragment.kt + NotificationsUtils.kt + WPCustomImageGetter.kt + WPRemoteResourceViewTarget.kt]
❓ I'm sharing the activity log detail screen that contains an image. But this screen shows the image as a link and doesn't use WPRemoteResourceViewTarget. I couldn't find a case that ActivityLogDetailFragment uses Glide. ActivityLogDetailFragment uses NotificationsUtilsWrapper, but it accesses only a function that is not related to Glide. So, is 5b related to Glide?
JP/WP Screenshot Testing [JPScreenshotTest.java + WPScreenshotTest.java + WPSupportUtils.java + PlaceholderComparison.java]
❓ Can't we say if CI tests pass, WPScreenshotTest and JPScreenshotTest still work fine?
|
👋 @irfano !
Awesome, thank you for reviewing and testing this, you rock! ❤️ 🙇 🚀
To my understanding this function is related to Glide, and that is, due to it using this addImageSpansForBlockMedia(...) function, which in turn, is using this WPCustomImageGetter class, which is using the WPRemoteResourceViewTarget class, and that in the end used Glide, thus me adding it to the testing list, just in case. Am I missing something here Irfan? 🤔 PS: I might surely do... 🤔 FYI: The reason for me adding these comprehensive test instruction is mainly twofold, on the one hand, just for us to be sure that this dependency, which is very outdated, will not cause any regression after being merged, and on the other hand, so that I can then use those testing instruction as the baseline for the test instructions documentation per dependency update work that I hope to deliver at some point in the future (see
Yes, for the purpose of this PR, I think we can, and thus this doesn't need to block this PR, but FYI and AFAIK the WP/JS screenshot tests are not running as part of the CI tests, so 🤷 ... |
There was a problem hiding this comment.
To my understanding this function is related to Glide, and that is, due to it using this addImageSpansForBlockMedia(...) function, which in turn, is using this WPCustomImageGetter class, which is using the WPRemoteResourceViewTarget class, and that in the end used Glide, thus me adding it to the testing list, just in case.
Am I missing something here Irfan? 🤔 PS: I might surely do... 🤔
Yes, you're right. ActivityLogDetailFragment can access Glide via WPCustomImageGetter, but I couldn't find a case that it can happen. I tested various activity log items, but mediaArray was null on all of them, so it skipped using WPCustomImageGetter.
So, there are 2 ways we can go. We can remove 5b from intructions or we keep it with ❓️ with an image ❓️ message, thus perhaps future testers may know a activity log detail type that can have mediaArray and trigger WPCustomImageGetter().
Since we're just discussing updating testing instructions, I'm approving the PR and leaving the merge to you.
Yes, for the purpose of this PR, I think we can, and thus this doesn't need to block this PR, but FYI and AFAIK the WP/JS screenshot tests are not running as part of the CI tests, so 🤷 ...
Oh yes! So we can skip testing WP/JP screenshot tests since they are broken.
|
👋 @irfano and thanks for your reply!
Same! 🫣
Yeap, I would vote for keeping it, at least for now and until we future out, at some point in the future, if that piece of logic can be tested or even doing anything. It might as well be dead code, who knows, this needs further and deep investigation but shouldn't block this PR... 🤷
Great, thanks for the approval and discussing all that with me, much appreciated that you took the time to do a thorough and detailed testing on this dependency update. ❤️ I'll proceed with merging now as everything else seems to be working anyway! 🚀
Yea! 🫣 |
Parent #17568
This PR updates
glideVersionto 4.15.1.PS: Since this library was anyway pointing to
4.12.0(it seems for some time now), the below commits was for documentation purposes only and weren't causing any actual changes whatsoever:PS: @irfano I added you as the main reviewer, randomly so, since I just wanted someone from the WordPress team to be aware of and sign-off on that change for WPAndroid. I also added the @wordpress-mobile/apps-infrastructure team, but this in done only for monitoring purposes, as such, I am not expecting any active review from that team. Thus, feel free to merge this PR if you deem so.
Dependency Refactor List:
To test:
1a. Post Editing Flow [libs:editor]
ℹ️ This test applies to both, the
WordPressandJetpackapps.❗️ This test makes sure that the
Post Editingflow, which comes from the libs:editormodule is also working as expected and that any transitive dependency changes aren't affecting
any
Materialrelated components.blogpost.paragraph,heading,image,video,separator,quote,gallery,columns,rowsblocks).Post Editingscreen, along with all the blocks you added, are shown andthat everything is functioning as expected.
1b. Story Flow [com.automattic:stories]
ℹ️ This test applies to the
Jetpackapp.❗️ This test makes sure that the
Storyflow, which comes from the com.automattic:storieslibrary is also working as expected and that any transitive dependency changes aren't affecting
any
Constraint Layoutrelated screens.storypost.imagesto that story.texton top of any image you uploaded.2a. Site Page Layout [ModalLayoutPickerFragment.kt + LayoutViewHolder.kt + ImageManager.kt]
ℹ️ This test applies to the
Jetpackapp.sitepage.Choose a layoutscreen is shown and functioning as expected,along with it displaying the layout thumbnails correctly.
2b. Site Creation Layout [HomePagePickerFragment.kt + LayoutViewHolder.kt + ImageManager.kt]
ℹ️ This test applies to the
Jetpackapp.Site Picker-> Click the+button -> ChoseCreate WordPress.com site.What's your website about?screen is shown and functioning as expected, along withdisplaying the topics thumbnails correctly.
Lifestyle).Choose a themescreen is shown and functioning as expected,along with it displaying the theme thumbnails correctly.
3. Blog Post Layout [EditPostActivity.kt + AztecEditorFragment.kt + AztecImageLoader.kt]
ℹ️ This test applies to both, the
WordPressandJetpackapps.❗️ In order to be able to switch off the
Block Editoroption, the testing site must be on abusiness plan, or on a plan that still supports the use of the
Classic Editoroption.My Sitetab ->MENUsub-tab.Managesection at the bottom and click on itsSite Settingsoption.Editorsection in the middle and switch off theUse Block Editoroption. This willmake sure that any new post will be created using the
Aztecclassic editor.blogpost.image.along with it displaying the image thumbnail correctly.
4. Stats Widget Layout [ViewsWidgetUpdater.kt + WidgetUtils.kt]
ℹ️ This test applies to the
Jetpackapp.Views this weekor any other such widget to the home screen.Site, thenColorand click theADD WIDGETbutton.along with it displaying the site thumbnail correctly.
5a. Comment Detail Screen [CommentDetailFragment.kt + CommentUtils.kt + WPCustomImageGetter.kt + WPRemoteResourceViewTarget.kt]
ℹ️ This test applies to both, the
JetpackandWordPressapps.❓️ Not sure how to best and quickly test this, let me know if you have an idea.
My Sitetab ->MENUsub-tab.Contentsection at the top and click on itsCommentsoptionto open the
Commentsscreen.Comment Detailscreen is shown and functioning as expected,along with it displaying the image thumbnail correctly.
5b. Activity Log Detail Screen [ActivityLogDetailFragment.kt + NotificationsUtils.kt + WPCustomImageGetter.kt + WPRemoteResourceViewTarget.kt]
ℹ️ This test applies to both, the
JetpackandWordPressapps.❓️ Not sure how to best and quickly test this, let me know if you have an idea.
My Sitetab ->MENUsub-tab.Managesection in the middle and click on itsActivity Logoption.Activity Log Detailscreen is shown and functioning as expected,along with it displaying the image thumbnail correctly.
6. JP/WP Screenshot Testing [JPScreenshotTest.java + WPScreenshotTest.java + WPSupportUtils.java + PlaceholderComparison.java]
ℹ️ This test applies to both, the
JetpackandWordPressapps.❗️ Not sure if it is worth it to test this, let me know if you have any thoughts on that.
❓️ Not sure how to best and quickly test this, let me know if you have an idea.
Regression Notes
Potential unintended areas of impact
Glideis a library that is being used across many screens, especially screens that utilize any image loading functionality.What I did to test those areas of impact (or what existing automated tests I relied on)
To testsection.What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txtif necessary.UI Changes testing checklist: