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

Temporal: Add tests for PlainTime string disambiguation with time zone #3641

Merged
merged 4 commits into from
Aug 31, 2022

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Aug 15, 2022

This implements the normative change in
tc39/proposal-temporal#2284 which reached
consensus at the July 2022 TC39 meeting.

It adds tests that ensure strings like HHMM-UU[TZ] and HHMMSS[TZ] do not
require a disambiguating T separator, even if HHMM-UU and HHMMSS would by
themselves.

@ptomato ptomato added the has consensus This has committee consensus label Aug 15, 2022
@ptomato ptomato requested a review from gibson042 August 15, 2022 23:36
@ptomato ptomato requested a review from a team as a code owner August 15, 2022 23:36
@ptomato
Copy link
Contributor Author

ptomato commented Aug 16, 2022

I've tacked tests for tc39/proposal-temporal#2287 onto this PR as well, since they are very similar.

gibson042
gibson042 previously approved these changes Aug 17, 2022
Copy link
Contributor

@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.

This looks like it works, but I am concerned about all these duplicate lists falling out of sync.

Ms2ger
Ms2ger previously approved these changes Aug 31, 2022
This implements the normative change in
tc39/proposal-temporal#2284 which reached
consensus at the July 2022 TC39 meeting.

It adds tests that ensure strings like HHMM-UU[TZ] and HHMMSS[TZ] do not
require a disambiguating T separator, even if HHMM-UU and HHMMSS would by
themselves.
This implements the normative change in
tc39/proposal-temporal#2287 which reached
consensus at the July 2022 TC39 meeting.

It adds tests that ensure that PlainTime strings which require a T
designator for disambiguation, are not disambiguated by adding a calendar
annotation.
…ndar

The strings with calendar annotations in these tests don't need to be
listed separately, they can be derived from the original strings.
Ms2ger
Ms2ger previously approved these changes Aug 31, 2022
…oralHelpers

This adds an object, TemporalHelpers.ISO, which has methods that return
arrays of various ISO strings. The idea is to deduplicate more string
tests into methods on this object.
@ptomato ptomato merged commit 051631f into tc39:main Aug 31, 2022
@ptomato ptomato deleted the temporal-2284 branch August 31, 2022 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants