-
Notifications
You must be signed in to change notification settings - Fork 107
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
Editorial: Adapt to ECMA-262 DefaultTimeZone #721
Merged
ryzokuken
merged 3 commits into
tc39:master
from
ptomato:adapt-to-ecma262-defaulttimezone
Nov 3, 2022
Merged
Editorial: Adapt to ECMA-262 DefaultTimeZone #721
ryzokuken
merged 3 commits into
tc39:master
from
ptomato:adapt-to-ecma262-defaulttimezone
Nov 3, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that ECMA-262 has gained DefaultTimeZone, note that the definition in this specification supersedes the other one. Add a structured header while we are at it.
The operation LocalTZA from ECMA-262 has been replaced by other, more clearly specified, mechanisms for determining the time zone offset. (The language "LocalTZA() where the local time zone is replaced with _timeZone_" seemed insufficiently specific, anyway.) This uses the new operation GetNamedTimeZoneOffsetNanoseconds from ECMA-262 to calculate the time zone offset. This operation encapsulates the requirement to "use best available information about the specified time zone, including current and historical information about time zone offsets from UTC and daylight saving time rules," so we can delete that text from the non-Gregorian branch in this operation. Add a structured header while we're at it. In addition, I believe the types of _t_ and _tz_ were backwards. _t_ is an exact time in milliseconds, so it is a time value. Elsewhere we ensure _t_ is not NaN, so we can say it is a finite time value. _tz_ may not be a time value once _offsetMs_ (formerly _timeZoneOffset_) is added to it, so it is an integral Number.
ptomato
changed the title
Adapt to ECMA-262 DefaultTimeZone
Editorial: Adapt to ECMA-262 DefaultTimeZone
Oct 10, 2022
@Ms2ger suggested I add a third commit that changes ToLocalTime to use epoch nanoseconds, which is similar to a change that we have in Temporal. This is also an editorial change and I think there's an advantage to bringing it in now. |
gibson042
requested changes
Oct 18, 2022
This is an optional third commit that changes ToLocalTime to use epoch nanoseconds instead of milliseconds, in anticipation of Temporal becoming part of the specification. However, there is a small advantage to bringing this change in now, because using epoch nanoseconds fits better with the adaptation to use GetNamedTimeZoneOffsetNanoseconds from the previous commit, which also takes epoch nanoseconds. I've taken the liberty of changing the "Operation(x) specified in es2023's Operation" language to just use the automatic linking through the ecma262 bibliography.
ptomato
force-pushed
the
adapt-to-ecma262-defaulttimezone
branch
from
October 18, 2022 19:18
3d90f0f
to
86c7db3
Compare
gibson042
approved these changes
Oct 18, 2022
ryzokuken
approved these changes
Nov 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes some editorial changes as a result of tc39/ecma262#2781:
Closes: #684