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

Normative: Add Promise.any & AggregateError #2040

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

chicoxyzzy
Copy link
Member

Specification text for https://github.com/tc39/proposal-promise-any

This also includes changes to IterableToList runtime semantics

spec.html Outdated Show resolved Hide resolved
@anba
Copy link
Contributor

anba commented Jun 10, 2020

Maybe it's time to move IterableToList to section 7.4. When I added it in #269, I've intentionally kept it buried deep down in %TypedArray%.from to make it less likely it's used incorrectly. (Incorrectly insofar that we don't want spec writers to use it to convert an iterable into a list and then iterate over that list to perform some action. Instead spec writers should use IteratorStep to consume iterables.)

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented Jun 10, 2020

Moving IterableToList to 7.4, as @anba proposed, seems reasonable to me

@ljharb ljharb added needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4. labels Jun 10, 2020
spec.html Outdated Show resolved Hide resolved
@mathiasbynens
Copy link
Member

Given https://github.com/tc39/test262/tree/master/test/built-ins/Promise/any and https://github.com/tc39/test262/tree/master/test/built-ins/NativeErrors/AggregateError, can the "needs tests" label be removed?

@ljharb ljharb added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Jun 24, 2020
ljharb pushed a commit to chicoxyzzy/ecma262 that referenced this pull request Jun 24, 2020
chicoxyzzy added a commit to chicoxyzzy/ecma262 that referenced this pull request Jul 2, 2020
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Co-authored-by: Kevin Gibbons <bakkot@gmail.com>
chicoxyzzy added a commit to chicoxyzzy/ecma262 that referenced this pull request Jul 2, 2020
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Co-authored-by: Kevin Gibbons <bakkot@gmail.com>
@chicoxyzzy chicoxyzzy closed this Jul 2, 2020
@chicoxyzzy chicoxyzzy deleted the promise-any branch July 2, 2020 13:18
@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented Jul 2, 2020

Oops that was wrong remote branch.

I'm trying to add @mathiasbynens and @bakkot as co-authors on copy of this branch in my fork of the repo (just to try if it works), but messed up this one and it's copy and deleted wrong one 😅

@chicoxyzzy chicoxyzzy restored the promise-any branch July 2, 2020 13:19
@chicoxyzzy chicoxyzzy reopened this Jul 2, 2020
chicoxyzzy added a commit to chicoxyzzy/ecma262 that referenced this pull request Jul 2, 2020
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Co-authored-by: Kevin Gibbons <bakkot@gmail.com>
@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented Jul 2, 2020

Now it's correct and synced with master

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@ljharb ljharb requested review from michaelficarra, syg, bakkot and a team July 8, 2020 21:52
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, just a few editorial convention issues.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@ljharb ljharb requested a review from a team July 14, 2020 21:37
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@ljharb ljharb requested a review from bakkot July 15, 2020 16:49
@ljharb ljharb self-assigned this Jul 15, 2020
@bakkot
Copy link
Contributor

bakkot commented Jul 19, 2020

@chicoxyzzy I agree that IterableToList should be moved to 7.4. Want to do that? Editors can take care of it when merging if you don't have time to get to it.

Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with the caveat that IterableToList should be relocated before merging.

@chicoxyzzy
Copy link
Member Author

@bakkot I put IterableToList at the end of 7.4.

@ljharb ljharb added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Jul 21, 2020
Co-authored-by: chicoxyzzy <chi187@gmail.com>
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Co-authored-by: Kevin Gibbons <bakkot@gmail.com>
@ljharb ljharb merged commit 69f5349 into tc39:master Jul 21, 2020
@chicoxyzzy chicoxyzzy deleted the promise-any branch July 21, 2020 18:25
ljharb pushed a commit to ExE-Boss/ecma262 that referenced this pull request Jul 21, 2020
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Jul 21, 2020
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Jul 22, 2020
ljharb added a commit to ljharb/ecma262 that referenced this pull request Aug 3, 2020
This is non-normative since it is not a function, and nothing uses it in
this or any other spec. It was mistakenly added in tc39#2040.
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 2, 2020
This is non-normative since it is not a function, and nothing uses it in
this or any other spec. It was mistakenly added in tc39#2040.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants