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

LibJS: Avoid double construction in Array.fromAsync #20819

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

shannonbooth
Copy link
Member

@shannonbooth shannonbooth commented Aug 28, 2023

This is a normative change in the array from async proposal, see:

tc39/proposal-array-from-async@49cfde2

It fixes a double construction when Array.fromAsync is given an array
like object.

Two test262 tests will now fail, as they have not (currently) been updated to match the updated spec.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 28, 2023
@shannonbooth shannonbooth marked this pull request as draft August 28, 2023 09:06
@shannonbooth
Copy link
Member Author

shannonbooth commented Aug 28, 2023

hmm one second, might have just seen an issue

@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 28, 2023
@shannonbooth shannonbooth marked this pull request as ready for review August 28, 2023 09:10
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 28, 2023
@shannonbooth
Copy link
Member Author

shannonbooth commented Aug 28, 2023

Nevermind, false alarm, I confused myself into thinking I was causing test262 failures, but these are expected failures as they were testing the old behaviour.

Copy link
Member

@trflynn89 trflynn89 left a comment

Choose a reason for hiding this comment

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

Minor note on the commit message, we typically format editorial/normative updates like so, linking to the spec commit rather than its PR:

This is a normative change in the Set Methods proposal. See:
https://github.com/tc39/proposal-set-methods/commit/4155e6e

See e.g. 4b0ef6b, 31e555a,

This is a normative change in the array from async proposal, see:

tc39/proposal-array-from-async@49cfde2

It fixes a double construction when Array.fromAsync is given an array
like object.
@shannonbooth
Copy link
Member Author

Minor note on the commit message, we typically format editorial/normative updates like so, linking to the spec commit rather than its PR:

This is a normative change in the Set Methods proposal. See:
https://github.com/tc39/proposal-set-methods/commit/4155e6e

See e.g. 4b0ef6b, 31e555a,

Thanks, fixed

@shannonbooth shannonbooth changed the title LibJS: Apply normative change to Array.fromAsync to fix double construct LibJS: Avoid double construction in Array.fromAsync Aug 28, 2023
@trflynn89 trflynn89 merged commit 9b884a9 into SerenityOS:master Aug 29, 2023
14 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants