Closed Pike closed 5 years ago
Merging #261 into master will increase coverage by
0.99%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #261 +/- ##
============================================
+ Coverage 35% 35.99% +0.99%
- Complexity 371 395 +24
============================================
Files 123 125 +2
Lines 3240 3273 +33
Branches 507 509 +2
============================================
+ Hits 1134 1178 +44
+ Misses 2010 1998 -12
- Partials 96 97 +1
Impacted Files | Coverage Δ | Complexity Δ | |
---|---|---|---|
.../java/org/mozilla/focus/browser/BrowserFragment.kt | 0% <0%> (ø) |
0% <0%> (ø) |
:arrow_down: |
...rg/mozilla/focus/home/NavigationOverlayFragment.kt | 0% <0%> (ø) |
0% <0%> (ø) |
:arrow_down: |
...main/java/org/mozilla/focus/ext/FragmentManager.kt | 0% <0%> (ø) |
0% <0%> (ø) |
:arrow_down: |
...pp/src/main/java/org/mozilla/focus/MainActivity.kt | 0% <0%> (ø) |
0% <0%> (ø) |
:arrow_down: |
...rg/mozilla/focus/session/SessionRestorerStorage.kt | 100% <0%> (ø) |
7% <0%> (?) |
|
.../java/org/mozilla/focus/session/SessionRestorer.kt | 93.33% <0%> (ø) |
8% <0%> (?) |
|
...lla/focus/session/VisibilityLifeCycleCallback.java | 29.16% <0%> (+4.16%) |
4% <0%> (+1%) |
:arrow_up: |
...rc/main/java/org/mozilla/focus/ScreenController.kt | 5.12% <0%> (+5.12%) |
2% <0%> (+2%) |
:arrow_up: |
...a/org/mozilla/focus/MainActivityIntentResponder.kt | 100% <0%> (+10%) |
9% <0%> (+2%) |
:arrow_up: |
...main/java/org/mozilla/focus/utils/LiveDataEvent.kt | 100% <0%> (+100%) |
1% <0%> (+1%) |
:arrow_up: |
... and 1 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 31317f9...52b786f. Read the comment docs.
@mcomella, I notice that
error_connectionfailure_message
and friends are in echo show, but not in tv. Is that something that wasn't obvious when you removed unused strings, or is that an actual difference between the apps?
FFTV uses components to display its error pages (which has the strings) and we haven't gotten around to that yet: likely fixed in #100.
Does it make sense to block on that? Or should we go ahead with the status quo for now?
Does it make sense to block on that? Or should we go ahead with the status quo for now?
@Pike Why do you think we would want to block? I don't see any reasons to do so.
@mcomella this is good for another round of review and hopefully landing.
Pull Request checklist
./gradlew connectedAmazonWebViewDebugAndroidTest
)qa-ready
orqa-denied
)Adding an l10n configuration file for echo show, and removing non-localizable strings from
strings.xml
.The existing strings trigger a ton of compare-locales errors (which doesn't run in automation yet anyway). That's OK, I expect us to fix this by doing a re-import from po-files into the android l10n repo, and then just land those fixes.
@mcomella, I notice that
error_connectionfailure_message
and friends are in echo show, but not in tv. Is that something that wasn't obvious when you removed unused strings, or is that an actual difference between the apps?CC @Delphine