Sub-minute time zone offsets #1035

ptomato · 2020-10-22T03:45:52Z

Closes: #935

This is necessary for round-tripping of some named time zones. In particular we are testing with Europe/Amsterdam before 1937. See: #935

Allows time zone offsets such as +00:19:32.13 in Temporal.Instant.from, Temporal.TimeZone.from, and new Temporal.TimeZone. See: #935

codecov · 2020-10-22T03:47:11Z

Codecov Report

Merging #1035 into main will decrease coverage by 0.00%.
The diff coverage is 97.87%.

@@            Coverage Diff             @@
##             main    #1035      +/-   ##
==========================================
- Coverage   94.33%   94.33%   -0.01%     
==========================================
  Files          18       18              
  Lines        6404     6420      +16     
  Branches      950      959       +9     
==========================================
+ Hits         6041     6056      +15     
- Misses        356      357       +1     
  Partials        7        7

Flag	Coverage Δ
#test262	`46.92% <71.42%> (+0.04%)`	⬆️
#tests	`90.36% <95.74%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
polyfill/lib/ecmascript.mjs	`96.09% <97.61%> (-0.01%)`	⬇️
polyfill/lib/regex.mjs	`100.00% <100.00%> (ø)`
polyfill/lib/timezone.mjs	`95.16% <100.00%> (-0.17%)`	⬇️

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 cef7855...7771b92. Read the comment docs.

ryzokuken · 2020-10-22T06:18:16Z

polyfill/test/timezone.mjs

+    const zone = new Temporal.TimeZone('Europe/Amsterdam');
+    const inst = Temporal.Instant.from('1900-01-01T12:00Z');
+    const dtm = Temporal.DateTime.from('1900-01-01T12:00');
+    it(`${zone} has ID ${zone}`, () => equal(zone.id, `${zone}`));


A feel a bit iffy about comparing two computed values, one of these should ideally be a constant, otherwise they could both be the wrong yet the same and it would pass. That said, it's a very minor nit.

This was already the case in the other tests surrounding this one, but maybe it's worth a follow up issue if you want to open one?

ptomato added 3 commits October 21, 2020 20:43

Output sub-minute time zone offset in toString methods

5db4ae6

This is necessary for round-tripping of some named time zones. In particular we are testing with Europe/Amsterdam before 1937. See: #935

Consolidate duplicate parseOffsetString()

ca4c270

Parse sub-minute time zone offset in ISO strings and time zone IDs

7771b92

Allows time zone offsets such as +00:19:32.13 in Temporal.Instant.from, Temporal.TimeZone.from, and new Temporal.TimeZone. See: #935

ptomato requested review from Ms2ger, justingrant, pipobscure, jasonwilliams, sffc, gibson042, mattjohnsonpint and ryzokuken October 22, 2020 03:45

ryzokuken approved these changes Oct 22, 2020

View reviewed changes

ryzokuken merged commit 76d2dab into main Oct 22, 2020

ryzokuken deleted the 935-sub-minute-offsets branch October 22, 2020 06:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sub-minute time zone offsets #1035

Sub-minute time zone offsets #1035

ptomato commented Oct 22, 2020

codecov bot commented Oct 22, 2020 •

edited

Loading

ryzokuken Oct 22, 2020

ptomato Oct 22, 2020

Sub-minute time zone offsets #1035

Sub-minute time zone offsets #1035

Conversation

ptomato commented Oct 22, 2020

codecov bot commented Oct 22, 2020 • edited Loading

Codecov Report

ryzokuken Oct 22, 2020

Choose a reason for hiding this comment

ptomato Oct 22, 2020

Choose a reason for hiding this comment

codecov bot commented Oct 22, 2020 •

edited

Loading