-
Notifications
You must be signed in to change notification settings - Fork 7
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
Reviewers for Stage 3 #17
Comments
I'm still not a fan of the term "identity" here - I believe tc39/ecma262#2405 specifically intends to only use that term with spec values and not ecmascript values. Otherwise, the only nit I have is that HasIdentity could say "is Object or Symbol," and be reduced to two lines (none of these should be stage 3 blockers, but I'd strongly advocate for changing the word to something less confusing before merging into the main spec) |
LGTM for Stage 3. I think we can iterate on the names of abstract algorithms after Stage 3. |
LGTM for Stage 3 modulo concerns from @ljharb |
Thanks! I have no strong preference over the abstraction name. HasIdentity was called this way to reflect general functionality but I also don't intend to expand this proposal to other types. One review item I received in private:
I don't have intention or goals to add other primitive values to weak collections and this remains out of scope for this proposal. Someone else might wanna discuss about Records and Tuples but it's not the case for this proposal. |
Almost 1 year later, we are once again in a position where we require spec review as we prepare to go for stage 3. As per #19 the full set of proposed changes is being done as a PR due to the proposal's need for many small changes (e.g. updating liveness to reference both objects and symbols). The key difference from last year is that 'registered symbols' (those created with cc: @mhofman @gibson042 @brad4d @ljharb @michaelficarra @bakkot @syg |
@acutmore Thanks for driving this! One pet peeve: we'd all benefit of a direct link to the rendered spec, but I'm also in favor of having this mentioned PR merged already for the said review. |
No worries! Good idea - I've switched things around so we now get a rendered preview here: https://arai-a.github.io/ecma262-compare/?pr=2777 |
Updated review (looking at the 262 PR) |
lgtm for stage 3 |
Hi @gibson042 & @ljharb I marked you both as reviewed here as Ashley did go through your comments. Let me know if that is not satisfying to you, I can rescind that review. @mhofman can you let us know if you had a chance to review? |
Tracking issue for spec reviews for advancement to Stage 3.
From notes: JHD, RGN, BSH,
DEMAHThe text was updated successfully, but these errors were encountered: