-
Notifications
You must be signed in to change notification settings - Fork 156
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
Fixes #848
Conversation
This fixes `npm run lint -- --fix`.
Codecov Report
@@ Coverage Diff @@
## main #848 +/- ##
=======================================
Coverage 92.35% 92.35%
=======================================
Files 17 17
Lines 4840 4841 +1
Branches 751 752 +1
=======================================
+ Hits 4470 4471 +1
Misses 363 363
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with @Ms2ger's comments addressed.
Although they were never used in any observable output, the polyfill's ParseISODateTime would generate offset strings like '+undefined:00' when parsing a string with a Z designator. Change the spec accordingly, to always make the result of parsing a time zone string with a Z equivalent to "+00:00[UTC]". Remove some duplicated logic.
GetEpochFromParts returns null if the DateTime fields are outside of the supported Date range. Callers need to check for a null return value.
If we change Absolute to use the offset and not the bracket name, then these tests will start failing.
This was an oversight in the spec.
@Ms2ger Can you check this one again? It can't be merged until you mark that the requested changes have been addressed. |
Oh, never mind, you can "dismiss review" if the comments have been addressed ... which strikes me as a bit rude though! |
Miscellaneous fixes.