-
Notifications
You must be signed in to change notification settings - Fork 159
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
Normative: Remove "60" from TimeMinuteNotValidDay #2279
Conversation
This is probably non-normative, because "60" is already rejected in `ParseISODateTime` when `IsValidTime` gets called. Fixes tc39#2238
Codecov Report
@@ Coverage Diff @@
## main #2279 +/- ##
=======================================
Coverage 91.06% 91.06%
=======================================
Files 19 19
Lines 10528 10528
Branches 1699 1699
=======================================
Hits 9587 9587
Misses 929 929
Partials 12 12
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.
I think it is normative:
- Temporal.Calendar.from ( calendarLike ) calls ToTemporalCalendar(calendarLike).
- ToTemporalCalendar(temporalCalendarLike) with non-object temporalCalendarLike stringifies it to identifier and ParseTemporalCalendarString(identifier) if it does not identify a built-in calendar.
- ParseTemporalCalendarString ( isoString ) parses isoString as a |TemporalCalendarString| and returns the contained |CalendarName| as a string or "iso8601" if there is no |CalendarName|.
- |TemporalCalendarString| can expand to |CalendarTime|, which can expand to |TimeSpecWithOptionalTimeZoneNotAmbiguous|, which can expand to |TimeHourMinuteBasicFormatNotAmbiguous| |TimeZoneBracketedAnnotation|?, in which |TimeHourMinuteBasicFormatNotAmbiguous| can expand to |TimeHour| |TimeMinuteNotValidDay|.
So per current spec, Temporal.Calendar.from("0160")
should return the built-in ISO 8601 calendar. But that is the very #2238 bug being fixed by this change.
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.
Well, dang. I have to agree with Richard that it is normative. I guess this one goes on the list for July, so I'll mark it as draft.
Subsumed by #2284. |
This is probably non-normative, because "60" is already rejected in
ParseISODateTime
whenIsValidTime
gets called.Fixes #2238