-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove experimental per app feature flag #21521
Remove experimental per app feature flag #21521
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/per-app-language-prefs #21521 +/- ##
===============================================================
Coverage 39.44% 39.44%
===============================================================
Files 2119 2119
Lines 99556 99556
Branches 15313 15313
===============================================================
Hits 39269 39269
Misses 56806 56806
Partials 3481 3481 ☔ View full report in Codecov by Sentry. |
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.
👋 @nbradbury !
I have reviewed and tested this PR as per the instructions, everything works as expected, great clean-up! 🧹 🧹 🧹
I have left few questions (❓)and some minor (🔍) comments for you to consider. I am going to approve this PR anyway, since none is blocking and I don't want to block progress on this. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.
WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsFragment.java
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsFragment.java
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsFragment.java
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/util/PerAppLocaleManager.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/prefs/ExperimentalFeaturesActivity.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/util/PerAppLocaleManager.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/util/PerAppLocaleManager.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/util/PerAppLocaleManager.kt
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
This PR removes the experimental feature flag for per app language prefs, which enables per app prefs for all users. This targets the
feature/per-app-language-prefs
feature branch rather thantrunk
because there's much more work to do before this feature can be released (most notably, dropping theLocaleAwareActivity
). In addition, we also want the experimental feature flag to be available in at least one release before removing it.Test 1:
Test 2: