-
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 locale aware activity #21523
Remove locale aware activity #21523
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress 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 #21523 +/- ##
===============================================================
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, good job! 🌟
EXTRAS
About this Fixed detekt errors commit and the TODO: Xyz
comment having its colon (:
) character removed just to fixed Detekt complaining, I suggest keeping that colon (:
) character, otherwise the TODO Xyz
format is not the expected one. Instead, I suggest adding the @Suppress("ForbiddenComment")
annotation to suppress this warning.
PS: Not related to this PR, but same applies to:
ReaderPostNewViewHolder.loadVideoThumbnail(...)
function (needs to suppress, it being a Kotlin class), andJobServiceId.isJobServiceWithSameParamsPending(...)
method (need not to suppress, it being a Java class).
Them being the only other TODO
I could find with the colon (:
) character.
@ParaskP7 Can you elaborate on this? I'm not sure why this would be an improvement. |
Sure @nbradbury , I have a few things on that:
|
Quality Gate passedIssues Measures |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
As a follow up to #21521, this PR removes
LocaleAwareActivity
. There are a lot of files changed in this PR, but this was because we extendedLocaleAwareActivity
in so many places. In this PR, I changed them all to extendAppCompatActivity
.Testing steps are the same as the prior PR.
Test 1:
Test 2:
Note: There's still quite a bit of cleanup to do related to locale handling (unnecessary functions, etc.), but I wanted to keep this PR dedicated to removing
LocaleAwareActivity
. Cleanup is being addressed in this draft PR.