This repository was archived by the owner on Jan 13, 2025. It is now read-only.
chore: Remove unnecessary default arguments that affect coverage #3522
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
All 389 screenshot tests passed for commit 81029e3 vs. |
Codecov Report
@@ Coverage Diff @@
## master #3522 +/- ##
=======================================
Coverage 98.41% 98.41%
=======================================
Files 120 120
Lines 5033 5033
Branches 619 619
=======================================
Hits 4953 4953
Misses 80 80
Continue to review full report at Codecov.
|
acdvorak
approved these changes
Sep 5, 2018
acdvorak
previously requested changes
Sep 5, 2018
|
I saw the %s go up on my end, I'll look at the absolute numbers... |
|
I'm not sure where you got your numbers... Before: 95.35% Branches 1270/1332 After: 95.49% Branches 1270/1330 This is fully intended, since I removed two uncovered branches. |
|
All 389 screenshot tests passed for commit b9c5abe vs. |
kfranqueiro
dismissed
acdvorak’s stale review
Coverage number changes are exactly as expected
adrianschmidt
pushed a commit
to Lundalogik/material-components-web
that referenced
this pull request
Sep 25, 2018
…erial-components#3522) (cherry picked from commit 8c25ed5)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This solves the mystery in #3516 (comment).
All of our foundation constructors follow the pattern of calling
super(Object.assign(MDCFooFoundation.defaultAdapter, adapter)), but two components in particular (three if you include the Dialog prototype) were also assigning an empty object as the default value, which caused coverage to decrease due to the default "branch" never being taken. The default assignment is not necessary, sinceObject.assigneffectively no-ops onundefinedsources anyway.