Closed kevinoid closed 3 years ago
Merging #348 (bed73ac) into master (bdf8984) will increase coverage by
0.83%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #348 +/- ##
==========================================
+ Coverage 93.15% 93.98% +0.83%
==========================================
Files 1 1
Lines 555 549 -6
==========================================
- Hits 517 516 -1
+ Misses 38 33 -5
Flag | Coverage Δ | |
---|---|---|
unit | 93.98% <100.00%> (+0.83%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
src/fake-timers-src.js | 93.98% <100.00%> (+0.83%) |
:arrow_up: |
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 bdf8984...bed73ac. Read the comment docs.
Thank you
Purpose (TL;DR) - mandatory
Fix inconsistent use of
Object.keys
andkeys
polyfill.Background (Problem in detail) - optional
While considering how to implement
Object.assign
, I noticed thatObject.keys
is used in some places and thekeys
conditional polyfill is used in others. This intent is unclear (although after reviewing the history I assume it was left over from previous compatibility requirements) and it seemed to present needless complexity and maintenance burden.Solution - optional
The current sinon compatibility specifies that "the source is written as ES5.1", which defines Object.keys. This PR replaces the two remaining uses of the polyfill and removes it to reduce.
Alternate Solution
If there is reason to keep the polyfill, I could send a PR to use it everywhere, move it to commons, and/or add comments to explain why the use is necessary.