Skip to content
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

Editorial bug fixes and polyfill refactors to match recent editorial changes #2214

Merged
merged 5 commits into from
May 23, 2022

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented May 21, 2022

This contains editorial changes to the spec text:

  • Formatting fix
  • More structured headers for abstract operations and improvements to their descriptions

Also some refactors in the polyfill code, to match recent editorial changes:

@ptomato ptomato requested review from Ms2ger and justingrant May 21, 2022 01:04
@codecov
Copy link

codecov bot commented May 21, 2022

Codecov Report

Merging #2214 (f93892c) into main (08a5ec9) will increase coverage by 1.18%.
The diff coverage is 77.32%.

@@            Coverage Diff             @@
##             main    #2214      +/-   ##
==========================================
+ Coverage   89.99%   91.17%   +1.18%     
==========================================
  Files          19       19              
  Lines       10901    10524     -377     
  Branches     1683     1688       +5     
==========================================
- Hits         9810     9595     -215     
+ Misses       1079      917     -162     
  Partials       12       12              
Flag Coverage Δ
test262 83.19% <98.19%> (-0.41%) ⬇️
tests 82.12% <63.77%> (+1.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
polyfill/lib/plaintime.mjs 67.98% <46.15%> (+10.62%) ⬆️
polyfill/lib/ecmascript.mjs 93.50% <77.05%> (-2.32%) ⬇️
polyfill/lib/duration.mjs 81.93% <100.00%> (-1.92%) ⬇️
polyfill/lib/instant.mjs 91.13% <100.00%> (-2.79%) ⬇️
polyfill/lib/plaindate.mjs 82.62% <100.00%> (+5.36%) ⬆️
polyfill/lib/plaindatetime.mjs 78.57% <100.00%> (+5.23%) ⬆️
polyfill/lib/plainyearmonth.mjs 76.53% <100.00%> (+8.56%) ⬆️
polyfill/lib/zoneddatetime.mjs 98.76% <100.00%> (+0.88%) ⬆️
polyfill/lib/calendar.mjs 94.39% <0.00%> (-0.02%) ⬇️

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 08a5ec9...f93892c. Read the comment docs.

Copy link
Collaborator

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec changes look good.

spec/abstractops.html Outdated Show resolved Hide resolved
spec/abstractops.html Outdated Show resolved Hide resolved
spec/abstractops.html Show resolved Hide resolved
spec/abstractops.html Show resolved Hide resolved
spec/abstractops.html Outdated Show resolved Hide resolved
ptomato added 5 commits May 23, 2022 12:42
PrepareTemporalFields, PreparePartialTemporalFields, and
MergeLargestUnitOption
Introduces the DRY operations ES.DifferenceTemporal___ in order to match
recent changes to the spec text.
This was previously done with the spread syntax, but we are going to make
a change to it, so we'll need a function instead. The spec text has the
operation MergeLargestUnitOption, so name it that.

The ObjectCreate(null) step was previously missing, so add it in order to
be compliant with the spec.
Introduces the DRY operations ES.AddDurationToOrSubtractDurationFrom___
in order to match recent changes to the spec text.
@ptomato ptomato merged commit 7671bd2 into main May 23, 2022
@ptomato ptomato deleted the editorial branch May 23, 2022 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants