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

Duration fixes #807

Merged
merged 4 commits into from Aug 10, 2020
Merged

Duration fixes #807

merged 4 commits into from Aug 10, 2020

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Aug 8, 2020

Minor bugs that I have run into while working on negative durations.

@codecov
Copy link

codecov bot commented Aug 8, 2020

Codecov Report

Merging #807 into main will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #807   +/-   ##
=======================================
  Coverage   92.24%   92.25%           
=======================================
  Files          17       17           
  Lines        4797     4800    +3     
  Branches      750      754    +4     
=======================================
+ Hits         4425     4428    +3     
  Misses        370      370           
  Partials        2        2           
Flag Coverage Δ
#test262 55.31% <33.33%> (-0.06%) ⬇️
#tests 87.04% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 94.95% <100.00%> (+<0.01%) ⬆️
polyfill/lib/regex.mjs 100.00% <100.00%> (ø)

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 1d80189...2150634. Read the comment docs.

polyfill/lib/regex.mjs Outdated Show resolved Hide resolved
polyfill/test/regex.mjs Outdated Show resolved Hide resolved
Copy link
Member

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. LGTM with @gibson042's comments addressed.

polyfill/test/regex.mjs Outdated Show resolved Hide resolved
ptomato and others added 4 commits August 10, 2020 11:42
These are not allowed according to ISO 8601, nor according to our
grammar in the spec, but we inadvertently accepted them and even tested
for them in the regex tests.
This was already the case in all of the types except Temporal.Duration.
Add tests to make sure it stays that way.
Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
@ptomato ptomato merged commit c929bd1 into main Aug 10, 2020
@ptomato ptomato deleted the duration-fixes branch August 10, 2020 19:02
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.

None yet

3 participants