FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps,util: V8: backport 74571c8 by BridgeAR · Pull Request #25941 · nodejs/node · GitHub

/ node Public

deps,util: V8: backport 74571c8 - #25941

Closed
BridgeAR wants to merge 2 commits into
nodejs:masterfrom
BridgeAR:backport-v8-fix
Closed

deps,util: V8: backport 74571c8#25941
BridgeAR wants to merge 2 commits into
nodejs:masterfrom
BridgeAR:backport-v8-fix

Conversation

BridgeAR commented Feb 5, 2019

Copy link
Copy Markdown
Member

Fixes: #24629

Please have a look at the commit messages for further details.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

BridgeAR requested a review from targos February 5, 2019 03:25

Copy link
Copy Markdown
Collaborator

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Feb 5, 2019

BridgeAR commented Feb 5, 2019
edited
Loading

Copy link
Copy Markdown
Member Author

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 5, 2019

BridgeAR commented Feb 5, 2019

Copy link
Copy Markdown
Member Author

@nodejs/v8-update PTAL

BridgeAR commented Feb 7, 2019
edited
Loading

Copy link
Copy Markdown
Member Author

Resumed CI https://ci.nodejs.org/job/node-test-commit/25687/ ✔️

Original commit message:

    Fix preview of set entries

    Set entries return an array with the value as first and second entry.
    As such these are considered key value pairs to align with maps
    entries iterator.
    So far the return value was identical to the values iterator and that
    is misleading.

    This also adds tests to verify the results and improves the coverage
    a tiny bit by testing different iterators.

    Refs: nodejs#24629

    R=yangguo@chromium.org

    Change-Id: I669a724bb4afaf5a713e468b1f51691d22c25253
    Reviewed-on: https://chromium-review.googlesource.com/c/1350790
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#59311}

Refs: v8/v8@74571c8
The inspection output for Set#entries() was wrong so far as it did
not return an array as it should have. That was a bug in V8 that is
now fixed and the code in Node.js has to be updated accordingly.

Copy link
Copy Markdown
Member Author

Rebased due to a conflict in the v8_embedder_string. Both CIs were green earlier and there was no other change, so I guess it should be fine without running new CIs.

It would be great to get another LG @nodejs/v8-update

pull Bot pushed a commit to Rachelmorrell/node that referenced this pull request Feb 20, 2019
Original commit message:

    Fix preview of set entries

    Set entries return an array with the value as first and second entry.
    As such these are considered key value pairs to align with maps
    entries iterator.
    So far the return value was identical to the values iterator and that
    is misleading.

    This also adds tests to verify the results and improves the coverage
    a tiny bit by testing different iterators.

    Refs: nodejs#24629

    R=yangguo@chromium.org

    Change-Id: I669a724bb4afaf5a713e468b1f51691d22c25253
    Reviewed-on: https://chromium-review.googlesource.com/c/1350790
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#59311}

Refs: v8/v8@74571c8

PR-URL: nodejs#25941
Fixes: nodejs#24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>
pull Bot pushed a commit to Rachelmorrell/node that referenced this pull request Feb 20, 2019
The inspection output for Set#entries() was wrong so far as it did
not return an array as it should have. That was a bug in V8 that is
now fixed and the code in Node.js has to be updated accordingly.

PR-URL: nodejs#25941
Fixes: nodejs#24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>

Copy link
Copy Markdown
Member Author

Landed in e557647 and 3d62d0c.

BridgeAR closed this Feb 20, 2019
addaleax pushed a commit that referenced this pull request Feb 21, 2019
Original commit message:

    Fix preview of set entries

    Set entries return an array with the value as first and second entry.
    As such these are considered key value pairs to align with maps
    entries iterator.
    So far the return value was identical to the values iterator and that
    is misleading.

    This also adds tests to verify the results and improves the coverage
    a tiny bit by testing different iterators.

    Refs: #24629

    R=yangguo@chromium.org

    Change-Id: I669a724bb4afaf5a713e468b1f51691d22c25253
    Reviewed-on: https://chromium-review.googlesource.com/c/1350790
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59311}

Refs: v8/v8@74571c8

PR-URL: #25941
Fixes: #24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit that referenced this pull request Feb 21, 2019
The inspection output for Set#entries() was wrong so far as it did
not return an array as it should have. That was a bug in V8 that is
now fixed and the code in Node.js has to be updated accordingly.

PR-URL: #25941
Fixes: #24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR mentioned this pull request Feb 26, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
Original commit message:

    Fix preview of set entries

    Set entries return an array with the value as first and second entry.
    As such these are considered key value pairs to align with maps
    entries iterator.
    So far the return value was identical to the values iterator and that
    is misleading.

    This also adds tests to verify the results and improves the coverage
    a tiny bit by testing different iterators.

    Refs: #24629

    R=yangguo@chromium.org

    Change-Id: I669a724bb4afaf5a713e468b1f51691d22c25253
    Reviewed-on: https://chromium-review.googlesource.com/c/1350790
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59311}

Refs: v8/v8@74571c8

PR-URL: #25941
Fixes: #24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
The inspection output for Set#entries() was wrong so far as it did
not return an array as it should have. That was a bug in V8 that is
now fixed and the code in Node.js has to be updated accordingly.

PR-URL: #25941
Fixes: #24629
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR deleted the backport-v8-fix branch January 20, 2020 11:53
This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL