Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment #19876

07jasjeet · 2024-01-03T09:36:22Z

Partially fixes #17253 and #17962

This PR include complete migration of NotificationsSettingsFragment to AndroidX implementation and migration of deprecated FragmentManager usages.
All the links and answers to some expected questions have been given in commits description itself.

A few mistakes to be considered while reading commit 275795f 's description:

Point 3 is duplicated from 2 but the actual message was:

Removed My Sites child preference screen in favour of recommended implementation.
The link mentioned is wrong, this is the intended link.

To Test:

Navigate to Notifications screen.
On top right corner, click on the settings logo.
Test EVERYTHING in this newly opened fragment

Regression Notes

Potential unintended areas of impact

Notifications Settings screen's child screens
What I did to test those areas of impact (or what existing automated tests I relied on)

Manual testing
What automated tests I added (or what prevented me from doing so)

None

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.

UI Changes Testing Checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
Talkback.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
Large and small screen sizes. (Tablet and smaller phones)
Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

1) Only updated feasible getFragmentManager() occurence

For java to kotlin conversion

Created AndroidX implementation of NotificationsSettingsDialogPreference.java

1) Ringtone preference is deprecated. 2) WPSwitchPreference replaced with AndroidX implementation.

1) Why? -> androidx.preference does not support addition of Child Preference Screens programtically no longer. Reference: https://stackoverflow.com/questions/60029806/why-does-the-nested-preferencescreen-not-open-when-using-androidx (Cites documentation as proof of result) 2) Why `ChildNotificationSettingsFragment`? -> To automatically change toolbar's state. 3) Why Change `NotificationsSettingsActivity`? -> The new androidx implementation requires the use of `PreferenceFragmentCompat` and to navigate to child preference screens, we need the parent activity to implement `PreferenceFragmentCompat.OnPreferenceStartFragmentCallback`

1) Why? -> androidx.preference does not support addition of Child Preference Screens programtically no longer. Reference: https://stackoverflow.com/questions/60029806/why-does-the-nested-preferencescreen-not-open-when-using-androidx (Cites documentation as proof of result) 2) Why `NotificationsMySitesSettingsFragment` interface? -> So, both `NotifcationsSettingsFragment` and a child preference screen deal with changes in settings for followed blogs/my sites. To separate out common code and give a form of abstraction to the purpose, this interface was implemented.

Why? PreferenceFragment has been deprecated in favour of PreferenceFragmentCompat Changes: 1) Previous java to kotlin migration fixes such as dependency injection fixes and null safety. 2) Removed Notification Types Child preference screen in favour of recommended implementation. 3) Removed Notification Types Child preference screen in favour of recommended implementation. 4) Removed Deprecated DialogPreference in favour of androidx implementation. 5) Ringtone Preference migration to Preference as support for it has been deprecated. References: https://stackoverflow.com/questions/60029806/why-does-the-nested-preferencescreen-not-open-when-using-androidx (cites high quality resource for problem and its solution)

07jasjeet · 2024-01-10T20:47:20Z

@antonis Pr is ready for review!

07jasjeet · 2024-01-10T20:59:45Z

Note: Component left in PR for easy review: NotificationsSettingsDialogPreference which should be removed before this PR is merged.

peril-wordpress-mobile · 2024-01-11T07:01:44Z

	Warnings
⚠️	PR is not assigned to a milestone.
⚠️	PR has more than 300 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

antonis · 2024-01-11T08:43:21Z

@antonis Pr is ready for review!

Thank you for your work on this @07jasjeet 🙇
My plan is to review this in detail next week.

I was wondering if the commits can be split into smaller PRs that can be reviewed and tested independently? If there are dependencies the PRs can be chained and merged one by one. We rarely land so big code changes in one batch.
Eg. could we split the refactoring or the Android X work #17962 (comment) into separate PRs.
The recommended size is <300 and anything above that triggers CI warning. On some occasions we may go above that but usually this involves test code.

Note: Component left in PR for easy review: NotificationsSettingsDialogPreference which should be removed before this PR is merged.

This can also be included if we extract the related work in a separate PR.

07jasjeet · 2024-01-11T09:03:51Z

I was wondering if the commits can be split into smaller PRs that can be reviewed and tested independently? If there are dependencies the PRs can be chained and merged one by one. We rarely land so big code changes in one batch. Eg. could we split the refactoring or the Android X work #17962 (comment) into separate PRs. The recommended size is <300 and anything above that triggers CI warning. On some occasions we may go above that but usually this involves test code.

Note: Component left in PR for easy review: NotificationsSettingsDialogPreference which should be removed before this PR is merged.

This can also be included if we extract the related work in a separate PR.

Alright, so each commit will have its own PRs. Considering that, we need some PRs as follows:

Migration of NotificationsSettingsDialogPreference. c7cbb31
With the branch of PR 1 as base, Creation of AndroidX implementation of WPSwitchPreference. 9372604
With the branch of PR 2 as base, Creation of Notification Types Fragment with Child fragment because both depend on each other. d2d3cb1
With the branch of PR 3 as base, Creation of My Sites Settings fragment. a61466b
With the branch of PR 4 as base, This commit cannot be split as everything in this commit is dependent on the previously mentioned PRs, 275795f
In this 5th PR alone, we would have
7bd1a5f and d9e40e2 as the code is intertwined with each other so much that things would break separately.

antonis · 2024-01-11T09:08:11Z

Alright, so each commit will have its own PRs.

The above PR split looks good to me 👍

antonis · 2024-01-11T10:50:20Z

Hey @07jasjeet 👋
As discussed on Slack I'll dive deeper on the PR changes and provide feedback on this PR next week before deciding on if/how to split your contribution in more PRs.

I've run some automated test and found the following.

Checkstyle

[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:85: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:95: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:32: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:48: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:89: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/models/NotificationsSettings.java:3:8: Unused import - androidx.annotation.NonNull. [UnusedImports]

Detekt

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:260:39: The function setupSwitchSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener) has too many parameters. The current threshold is set to 6. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:282:33: The function setupSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener?, onClickListener: View.OnClickListener?) has too many parameters. The current threshold is set to 6. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:59:18: The function onCreateDialog is too long (61). The maximum length is 60. [LongMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:174:17: The function configureLayoutForView appears to be too complex based on Cyclomatic Complexity (complexity: 16). Defined complexity threshold for methods is set to '15' [CyclomaticComplexMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:111:17: The function configureFollowedBlogsSettings is too long (63). The maximum length is 60. [LongMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:22:9: The function onMySiteSettingsChanged is too long (77). The maximum length is 60. [LongMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogPreferenceX.kt:8:45: The constructor(context: Context, attrs: AttributeSet?, channel: NotificationsSettings.Channel, type: NotificationsSettings.Type, blogId: Long, settings: NotificationsSettings, listener: NotificationsSettingsDialogPreference.OnNotificationsSettingsChangedListener, bloggingRemindersProvider: NotificationsSettingsDialogPreference.BloggingRemindersProvider?, dialogTitleRes: Int) has too many parameters. The current threshold is set to 7. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:587:17: The function configureFollowedBlogsSettings is too long (85). The maximum length is 60. [LongMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:75:7: Class NotificationsSettingsFragment is too large. Consider splitting it into smaller pieces. [LargeClass]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:527:17: The function configureBlogsSettings appears to be too complex based on Cyclomatic Complexity (complexity: 16). Defined complexity threshold for methods is set to '15' [CyclomaticComplexMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:587:17: The function configureFollowedBlogsSettings appears to be too complex based on Cyclomatic Complexity (complexity: 18). Defined complexity threshold for methods is set to '15' [CyclomaticComplexMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:132:85: This empty block of code can be removed. [EmptyFunctionBlock]
WordPress/src/main/java/org/wordpress/android/ui/prefs/WPSwitchPreferenceX.kt:83:17: Function getSwitch has 3 return statements which exceeds the limit of 2. [ReturnCount]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsTypesFragment.kt:153:1: Line detected, which is longer than the defined maximum line length in the code style. [MaxLineLength]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/ChildNotificationSettingsFragment.kt:30:35: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:35:1: Line detected, which is longer than the defined maximum line length in the code style. [MaxLineLength]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:127:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:107:1: Line detected, which is longer than the defined maximum line length in the code style. [MaxLineLength]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:110:1: Line detected, which is longer than the defined maximum line length in the code style. [MaxLineLength]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogPreferenceX.kt:7:58: There should be exactly one empty line in between the list of imports and the declaration of NotificationsSettingsDialogPreferenceX. [SpacingBetweenPackageAndImports]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:611:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:505:1: Line detected, which is longer than the defined maximum line length in the code style. [MaxLineLength]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:72:27: There should be exactly one empty line in between the list of imports and the declaration of NotificationsSettingsFragment. [SpacingBetweenPackageAndImports]

07jasjeet · 2024-01-11T17:06:37Z

Checkstyle

[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:85: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:95: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:32: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:48: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsMySitesSettingsFragment.kt:89: Specific imports should be used for FluxC Store inner classes [RegexpSingleline]
[ant:checkstyle] [ERROR] WordPress/src/main/java/org/wordpress/android/models/NotificationsSettings.java:3:8: Unused import - androidx.annotation.NonNull. [UnusedImports]

Not sure on how to solve the Specific imports should be used for FluxC Store inner classes check style issue. This was being used before in older implementations without any warnings but that is not the case here. Can you educate me about what this warning actually means?

antonis · 2024-01-12T08:05:15Z

Not sure on how to solve the Specific imports should be used for FluxC Store inner classes check style issue. This was being used before in older implementations without any warnings but that is not the case here.

Good question @07jasjeet 👍

The specific rule is related to how we import WordPress-FluxC-Android inner classes. It can be handled by using the full import of the inner class (e.g. in NotificationsSettingsFragment.java)

I've prepared a patch for the specific cases below.
RegexpSingleline.patch

This rule may seem opinionated but it is part of the Android Code Style Guidelines for Contributors that we are using.

On running detekt again, 8 unavoidable warning show up which existed prior to this PR's changes.

antonis · 2024-01-18T15:06:05Z

Hey @07jasjeet 👋
thank you for the submitted fixes. All the checkstyle issues have been resolved and most of the Detekt issue.
The following Detekt issues persist. Let me know if I can help with those 🙇

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:253:39: The function setupSwitchSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener) has too many parameters. The current threshold is set to 6. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:272:33: The function setupSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener?, onClickListener: View.OnClickListener?) has too many parameters. The current threshold is set to 6. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:170:17: The function configureLayoutForView appears to be too complex based on Cyclomatic Complexity (complexity: 16). Defined complexity threshold for methods is set to '15' [CyclomaticComplexMethod]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogPreferenceX.kt:9:45: The constructor(context: Context, attrs: AttributeSet?, channel: NotificationsSettings.Channel, type: NotificationsSettings.Type, blogId: Long, settings: NotificationsSettings, listener: NotificationsSettingsDialogPreference.OnNotificationsSettingsChangedListener, bloggingRemindersProvider: NotificationsSettingsDialogPreference.BloggingRemindersProvider?, dialogTitleRes: Int) has too many parameters. The current threshold is set to 7. [LongParameterList]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:74:7: Class NotificationsSettingsFragment is too large. Consider splitting it into smaller pieces. [LargeClass]
WordPress/src/main/java/org/wordpress/android/ui/prefs/WPSwitchPreferenceX.kt:83:17: Function getSwitch has 3 return statements which exceeds the limit of 2. [ReturnCount]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:134:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]
WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:632:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]

I will iterate with more feedback on the PR.

07jasjeet · 2024-01-18T15:12:17Z

Hey @07jasjeet 👋

thank you for the submitted fixes. All the checkstyle issues have been resolved and most of the Detekt issue.

The following Detekt issues persist. Let me know if I can help with those 🙇


WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:253:39: The function setupSwitchSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener) has too many parameters. The current threshold is set to 6. [LongParameterList]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:272:33: The function setupSettingView(settingName: String, settingValue: String?, settingSummary: String?, isSettingChecked: Boolean, isSettingLast: Boolean, onCheckedChangeListener: CompoundButton.OnCheckedChangeListener?, onClickListener: View.OnClickListener?) has too many parameters. The current threshold is set to 6. [LongParameterList]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:170:17: The function configureLayoutForView appears to be too complex based on Cyclomatic Complexity (complexity: 16). Defined complexity threshold for methods is set to '15' [CyclomaticComplexMethod]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogPreferenceX.kt:9:45: The constructor(context: Context, attrs: AttributeSet?, channel: NotificationsSettings.Channel, type: NotificationsSettings.Type, blogId: Long, settings: NotificationsSettings, listener: NotificationsSettingsDialogPreference.OnNotificationsSettingsChangedListener, bloggingRemindersProvider: NotificationsSettingsDialogPreference.BloggingRemindersProvider?, dialogTitleRes: Int) has too many parameters. The current threshold is set to 7. [LongParameterList]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:74:7: Class NotificationsSettingsFragment is too large. Consider splitting it into smaller pieces. [LargeClass]

WordPress/src/main/java/org/wordpress/android/ui/prefs/WPSwitchPreferenceX.kt:83:17: Function getSwitch has 3 return statements which exceeds the limit of 2. [ReturnCount]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsMySitesFragment.kt:134:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]

WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:632:14: The destructuring declaration contains 4 but only 3 are allowed. [DestructuringDeclarationWithTooManyEntries]

I will iterate with more feedback on the PR.

Hello @antonis
These detekt issues were present before my PR as well, as stated in the last commit's description.

These are unavoidable in my eye. I do need some suggestions as to how we should move forward with this.

antonis · 2024-01-18T17:05:06Z

Thank you for your quick response @07jasjeet and your contribution 🙇

These detekt issues were present before my PR as well, as stated in the last commit's description.

My understanding is that those are related to new code introduced in this PR. Probably the checks on the equivalent java code were different or suppressed.

These are unavoidable in my eye. I do need some suggestions as to how we should move forward with this.

For the two LongParameterList in the NotificationsSettingsDialogFragment we could create a data class to hold the settings related values. E.g.

    data class NotificationSetting(
        val settingName: String,
        val settingValue: String?,
        val settingSummary: String?,
        val isSettingChecked: Boolean,
        val isSettingLast: Boolean
    )

We can even add some default values and initialise this only with the properties needed on each case.

For the CyclomaticComplexMethod in NotificationsSettingsDialogFragment I would suggest splitting the method into more than one.

For the LongParameterList in NotificationsSettingsDialogPreferenceX we could create a data class like above.

The NotificationsSettingsFragment (LargeClass error) replaced an even larger Java class. Are there opportunities to split some of the functionality in a different class (e.g. a utility class)?

Regarding the DestructuringDeclarationWithTooManyEntries in NotificationsSettingsMySitesFragment and NotificationsSettingsFragment we could use the models directly. E.g.

for (model in models) {
//..
prefScreen.title = model.title
//...

or

models.forEach { 
 //...
prefScreen.title = it.title
 //...
}

The ReturnCount on the recursive getSwitch function of WPSwitchPreferenceX can either be suppressed or come up with a better way to implement this in Kotlin.

Trying to compile the code I also get the following warnings which are treated as errors in our configuration.

w: WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:234:13 Parameter 'compoundButton' is never used, could be renamed to _
w: WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:247:13 Parameter 'v' is never used, could be renamed to _
w: WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsDialogFragment.kt:287:51 Parameter 'v' is never used, could be renamed to _
w: WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.kt:121:9 'setHasOptionsMenu(Boolean): Unit' is deprecated. Deprecated in Java

One other generic issue not caught by the tools is the use of not-null assertion operator (!!) in many places in the introduced code. Though this may be introduced when converting code to Kotlin we should try to avoid it.

Taking a step back I think that we should return in our plan to split the code into multiple PRs that can be reviewed and tested individually. Do we need all the changes for #17253 or we enlarged the scope to also cover #17962?

07jasjeet · 2024-01-18T17:31:50Z

Taking a step back I think that we should return in our plan to split the code into multiple PRs that can be reviewed and tested individually. Do we need all the changes for #17253 or we enlarged the scope to also cover #17962?\

Alright Ill try to split commits as much as I can.

As for the rest, I'll not make commits to this PR anymore and rather deal with these detekt issues in separate the PRs.

irfano · 2024-06-17T15:02:32Z

Hey @07jasjeet! Would you still like to work on this? Should we keep this PR open?

jkmassel · 2024-07-10T15:08:07Z

Closing due to inactivity, but we would be happy to review further contributions in the future!

07jasjeet added 4 commits January 2, 2024 20:54

Update NotificationsSettingsFragment.java

e4e0dae

1) Only updated feasible getFragmentManager() occurence

Updated NotificationSettingsFragment with kt extenstion

357ce6b

For java to kotlin conversion

AS Auto-convert results

d2acb78

Fixed AS auto-convert errors

7633455

07jasjeet marked this pull request as draft January 10, 2024 11:17

07jasjeet added 8 commits January 11, 2024 01:35

Migrated NotificationsSettingsDialogPreference

c7cbb31

Created AndroidX implementation of NotificationsSettingsDialogPreference.java

Created AndroidX implementation of WPSwitchPreference

9372604

Updated Notification Settings Preference Layout

d9e40e2

1) Ringtone preference is deprecated. 2) WPSwitchPreference replaced with AndroidX implementation.

Migrated NotificationSettingsFollowedDialog to AndroidX

7bd1a5f

Dependency injection declaration for new child preference fragments

4015eb2

07jasjeet marked this pull request as ready for review January 10, 2024 20:46

07jasjeet changed the title ~~Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager~~ Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment Jan 11, 2024

antonis self-requested a review January 11, 2024 08:23

antonis added Tech Debt [Type] Task labels Jan 11, 2024

07jasjeet added 2 commits January 13, 2024 02:16

Check-style issues resolved

686ceaf

Partially solved detekt warnings

409d8b9

On running detekt again, 8 unavoidable warning show up which existed prior to this PR's changes.

antonis removed their request for review April 5, 2024 08:15

jkmassel closed this Jul 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment #19876

Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment #19876

07jasjeet commented Jan 3, 2024 •

edited

Loading

07jasjeet commented Jan 10, 2024

07jasjeet commented Jan 10, 2024

peril-wordpress-mobile bot commented Jan 11, 2024 •

edited

Loading

antonis commented Jan 11, 2024 •

edited

Loading

07jasjeet commented Jan 11, 2024

antonis commented Jan 11, 2024

antonis commented Jan 11, 2024 •

edited

Loading

07jasjeet commented Jan 11, 2024

Checkstyle

antonis commented Jan 12, 2024 •

edited

Loading

antonis commented Jan 18, 2024

07jasjeet commented Jan 18, 2024

antonis commented Jan 18, 2024 •

edited

Loading

07jasjeet commented Jan 18, 2024

irfano commented Jun 17, 2024

jkmassel commented Jul 10, 2024

Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment #19876

Part-4: Compiler Warnings as Errors - WordPress Module - FragmentManager and PreferenceFragment #19876

Conversation

07jasjeet commented Jan 3, 2024 • edited Loading

Partially fixes #17253 and #17962

To Test:

Regression Notes

PR Submission Checklist:

UI Changes Testing Checklist:

07jasjeet commented Jan 10, 2024

07jasjeet commented Jan 10, 2024

peril-wordpress-mobile bot commented Jan 11, 2024 • edited Loading

antonis commented Jan 11, 2024 • edited Loading

07jasjeet commented Jan 11, 2024

antonis commented Jan 11, 2024

antonis commented Jan 11, 2024 • edited Loading

Checkstyle

Detekt

07jasjeet commented Jan 11, 2024

Checkstyle

antonis commented Jan 12, 2024 • edited Loading

antonis commented Jan 18, 2024

07jasjeet commented Jan 18, 2024

antonis commented Jan 18, 2024 • edited Loading

07jasjeet commented Jan 18, 2024

irfano commented Jun 17, 2024

jkmassel commented Jul 10, 2024

07jasjeet commented Jan 3, 2024 •

edited

Loading

peril-wordpress-mobile bot commented Jan 11, 2024 •

edited

Loading

antonis commented Jan 11, 2024 •

edited

Loading

antonis commented Jan 11, 2024 •

edited

Loading

antonis commented Jan 12, 2024 •

edited

Loading

antonis commented Jan 18, 2024 •

edited

Loading