-
Notifications
You must be signed in to change notification settings - Fork 37
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
[Nullability Annotations to Java Classes] Add Missing Nullability Annotations to TaxonomyXMLRPCClient
(safe
)
#2866
[Nullability Annotations to Java Classes] Add Missing Nullability Annotations to TaxonomyXMLRPCClient
(safe
)
#2866
Conversation
Error: "'switch' statement on enum type 'org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType' misses cases: 'TIMEOUT', 'NO_CONNECTION', 'NETWORK_ERROR', 'NOT_FOUND', 'CENSORED', 'SERVER_ERROR', 'INVALID_SSL_CERTIFICATE', 'HTTP_AUTH_ERROR', 'INVALID_RESPONSE', 'NOT_AUTHENTICATED', 'PARSE_ERROR' and 'UNKNOWN'"
Warning: "Condition 'response != null' covered by subsequent condition 'response instanceof Map'"
Warning: "Redundant boxing, 'Long.parseLong()' call can be used instead"
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
FYI: 'n-a' stands for 'nullability annotations'.
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.
I've tested the PR on the FluxC example app and Jetpack (but not on WP to save time). I haven't noticed any issues. 🟢
I only have a comment for you to address.
.../src/main/java/org/wordpress/android/fluxc/network/xmlrpc/taxonomy/TaxonomyXMLRPCClient.java
Show resolved
Hide resolved
👋 @irfano and thank you so much for reviewing, testing and commenting on this PR, you rock! 🙇 ❤️ FYI: I left this comment for your that hopefully clears-up this confusion on why I made that change. |
Thanks for approving and merging this @irfano ! 🙇 ❤️ 🚀 |
Parent #2798
This PR adds any missing nullability annotation to TaxonomyXMLRPCClient.java.
FYI.1: This change is
safe
, meaning that there shouldn't be any (major) compile-time changes associated with this change. Having said that, testing is highly recommended since the changes in this PR can affect one or more clients (ie. JP/WPAndroid, WCAndroid, etc)FYI.2: An analysis on
TaxonomyStore
and its usages with our main clients suggests that this store is only being utilizing JP/WPAndroid. AFAIA WCAndroid is not using that store and as such can be safely ignored from testing.PS: @irfano I added you as the main reviewer, not so randomly, due to the fact that you've also reviewed #2826, and I wanted you from the Jetpack/WordPress mobile team again to be aware of and sign-off on that change for JP/WPAndroid. Feel free to merge this PR directly yourself if you deem so.
Nullability Annotation List:
Nullability Checks List:
Warnings Resolutions List:
Refactor List:
Associated Clients
It is highly recommending that this change is being tested on the below associated clients:
To Test:
FluxC Example
app via theTAXONOMIES
screen. Verify everything is working as expected.TaxonomyStore
related functionality on both, the WordPress and Jetpack apps, and see if everything is working as expected. For a couple of examples, you can expand and follow the inner and more explicit test steps within:1. Categories Settings Screen [CategoriesListFragment.kt]
ℹ️ This test applies to both, the
⚠️ It seems that this setting is not available for self-hosted WordPress sites.
Jetpack
andWordPress
app.Menu
sub-tab on the initialMy Site
screen and click onSite Settings
under theManage
section.Categories
under theWriting
section and click on it.Categories
screen is displayed and that everything works as expected.2. Tags Settings Screen [SiteSettingsTagListActivity.kt]
ℹ️ This test applies to both, the
⚠️ It seems that this setting is not available for self-hosted WordPress sites.
Jetpack
andWordPress
app.Menu
sub-tab on the initialMy Site
screen and click onSite Settings
under theManage
section.Tags
under theWriting
section and click on it.Tags
screen is displayed and that everything works as expected.3. Prepublishing Screens [PrepublishingTagsFragment.kt + PrepublishingCategoriesFragment.kt]
ℹ️ This test applies to both, the
Jetpack
andWordPress
app.Posts
screen and edit any post.UPDATE
button to have the bottom sheet appear (top-right
).Tags
row and add, remove or edit any tag for this post. Click back.Categories
row and add, remove or create a new category for this post. Click back.UPDATE NOW
button to update this post (bottom
).Tags
/Categories
are updated and that everything works as expected.