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

src: stop calling deprecated noop method by MarshallOfSound · Pull Request #31774 · nodejs/node · GitHub

/ node Public

src: stop calling deprecated noop method - #31774

Closed
MarshallOfSound wants to merge 1 commit into
nodejs:masterfrom
MarshallOfSound:patch-3
Closed

src: stop calling deprecated noop method#31774
MarshallOfSound wants to merge 1 commit into
nodejs:masterfrom
MarshallOfSound:patch-3

Conversation

Copy link
Copy Markdown
Member

This method was removed in V8 here https://chromium-review.googlesource.com/c/v8/v8/+/2033169 and has been a noop for a while

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 13, 2020
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 16, 2020

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member

@MarshallOfSound this does not appear to be a noop after all:

08:05:55 not ok 2295 parallel/test-v8-serdes
08:05:55   ---
08:05:55   duration_ms: 0.95
08:05:55   severity: fail
08:05:55   exitcode: 1
08:05:55   stack: |-
08:05:55     /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/test/parallel/test-v8-serdes.js:48
08:05:55         assert.deepStrictEqual(des.readValue(), obj);
08:05:55                                    ^
08:05:55     
08:05:55     Error: Unable to deserialize cloned data.
08:05:55         at Object.<anonymous> (/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/test/parallel/test-v8-serdes.js:48:32)
08:05:55         at Module._compile (internal/modules/cjs/loader.js:1206:30)
08:05:55         at Object.Module._extensions..js (internal/modules/cjs/loader.js:1226:10)
08:05:55         at Module.load (internal/modules/cjs/loader.js:1055:32)
08:05:55         at Function.Module._load (internal/modules/cjs/loader.js:950:14)
08:05:55         at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
08:05:55         at internal/main/run_main_module.js:17:47

failed deterministically across platforms

addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 3, 2020

nodejs-github-bot commented Apr 25, 2020
edited by addaleax
Loading

Copy link
Copy Markdown
Collaborator

targos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It is a noop on master now

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label May 2, 2020

addaleax commented May 9, 2020

Copy link
Copy Markdown
Member

Looks like this already happened in #32116.

addaleax closed this May 9, 2020
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. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.


Back | FazBrowse Home | New Git URL