Editor reviews

[bakkot]

I saw this was added to the agenda for this meeting. Just as a heads up, I don't think I'll have time to do a thorough review of the spec text before the meeting, especially given the amount of churn there's been. Can you let me (and the other editors - @tc39/ecma262-editors) know when it's camera-ready?

[ljharb]

Some very initial reactions, as i click around the spec randomly:

1. https://tc39.es/proposal-temporal/#sec-temporal-systemtimezone depends on SystemTimeZone, which is not an abstract operation in 262, but rather in 402. Temporal must work in the complete absence of 402.
2. SystemTimeZone uses CreateTemporalTimeZone, which is defined, but it links to the one in 402, which makes it very confusing to review. Can that be fixed?
3. https://tc39.es/proposal-temporal/#sec-temporal-timezonefrom observably looks up the from property on Temporal.TimeZone. This doesn't enable any subclassing nor does it make builtins robust against userland modification. The undefined fallback only protects against deletion, not replacement. Also https://tc39.es/proposal-temporal/#sec-temporal-calendarfrom, and presumably a bunch of other places.
4. https://tc39.es/proposal-temporal/#sec-temporal-createtemporaldate only requires that calendar be an Object, it does not validate that it has the internal slots of a Temporal.Calendar - it should. This appears to also be the case for all the other constructors that accept a calendar. Additionally, all the abstract operations that accept one should assert their type in some way.
5. similarly, https://tc39.es/proposal-temporal/#sec-temporal-createtemporalzoneddatetime does not validate that timeZone is actually a TimeZone object, only that it is an Object.
6. ToInteger is now named ToIntegerOrInfinity (altho the links still work, it would ideally be renamed)
7. https://tc39.es/proposal-temporal/#sec-temporal.duration.from gets the constructor, but does not use SpeciesConstructor. While that remains in the spec, it should. I assume this is the same on all the from methods.
8. Why does https://tc39.es/proposal-temporal/#sec-temporal-calendartostring exist at all? ToString(calendar) should achieve the same thing as long as the calendar extends Temporal.Calendar (which, per above, it should).

If any of these require discussion, please let me know, and I'll file issues.

[ptomato]

Thanks for reviewing @ljharb, I will respond to the question in the original post first and then to your review points!

The spec has been "camera-ready" since the last TC39 meeting. As far as I'm aware, the changes since November have been only to fix bugs and omissions in the spec that we have found in our own final reviews done by members of the champions group. My understanding was not that we had to leave bugs unfixed during this period, or that fixing bugs would constitute churn, but I apologize for any confusion about the state of the proposal that this caused.

That said, today we weren't able to come to a resolution in the champions group about #1203 (another issue that we identified during champions' own reviews) and since that does mean a bit larger change landing after the agenda deadline, I'll be withdrawing the request to go to Stage 3 in January, and put it on the agenda for the following meeting instead. In the January plenary we'll do a short update presentation and emphasize again that reviews are welcome.

Since the champions have finished our final reviews now, in order to avoid clouding the issue in the future, we intend to only act on review comments from other delegates after next week. I hope that will help!

[Ms2ger]

@ljharb thanks for the comments; please file issues for all of them.

[ljharb]

Done.

[ptomato]

Nothing left in this thread to do.