-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
Revert "v8: drop v8::FunctionCallbackInfo<T>::NewTarget()" #11652
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Is there related discussion context somewhere that is not linked to? |
@Fishrock123 Is #9293 (comment) what you are looking for? |
@ofrobots From #9293 (comment):
I don't think I can do that. |
b3f32f9
to
e71fc70
Compare
c2e818d
to
426968d
Compare
Ping @bnoordhuis ... other than needing a rebase, is this ready to land? |
6b8f3c7
to
1f0da51
Compare
Correct. Rebased although I don't understand why GH was showing a conflict, it applies cleanly. |
Yeah, I've also seen recently cases where github shows no conflicts for PRs that don't land cleanly. Not sure what's happening there. New CI before landing: https://ci.nodejs.org/job/node-test-pull-request/7002/ |
@bnoordhuis ... odd build failure on one of the linux boxes: https://ci.nodejs.org/job/node-test-commit-linux/8645/nodes=ubuntu1204-clang341-64/console |
@nodejs/build Is that perhaps an artifact from upgrading the compiler toolchain recently? The file it's complaining about, deps/v8/src/base/functional.cc, hasn't been touched by this PR. In fact, it hasn't been touched since November 2014. |
Yeah, that's why it's odd ;) Im not really sure why that would fail all of a sudden. |
Original Commit Message: PR-URL: nodejs#11872 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Backport-Of: nodejs#11872 PR-URL: nodejs#11943 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#11943 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#11670 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
These changes result in ~50% improvement in the included benchmark. PR-URL: nodejs#10580 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
59b869e
to
078188d
Compare
Backport-PR-URL: nodejs#11775 PR-URL: nodejs#10685 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Manually fix issues that eslint --fix couldn't do automatically. Backport-PR-URL: nodejs#11775 PR-URL: nodejs#10685 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Adds a centered logo to the README to make it a little more festive. As centering is not possible in pure Markdown, a bit of HTML is used. PR-URL: nodejs#12148 Ref: nodejs#6920 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
At least starting with Darwin Kernel Version 16.4.0, sending a SIGTERM to a process that is still starting up kills it with SIGKILL instead of SIGTERM. PR-URL: nodejs#12159 Refs: libuv/libuv#1226 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#12163 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
not needed according to official python docs - https://docs.python.org/2/library/subprocess.html#index-2 PR-URL: nodejs#12138 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
look for the actual produced `exe` not just the directory PR-URL: nodejs#12120 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
PR-URL: nodejs#11635 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: nodejs#12221 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
* Replace `var` by `const`. * Comment out ellipses. * Update code example (provide relevant file path, add missing option). PR-URL: nodejs#12171 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I noticed that few of the print statements in configure have a leading space with is not consistent with the rest of the file. Not sure if this intentional or not so creating this commit just to bring it up just in case. PR-URL: nodejs#12176 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Currently it is possible to configure using --without-ssl and --shared-openssl/--openssl-no-asm/openssl-fips without an error occuring. The commit add check for these combinations: $ ./configure --without-ssl --shared-openssl Error: --without-ssl is incompatible with --shared-openssl $ ./configure --without-ssl --openssl-no-asm Error: --without-ssl is incompatible with --openssl-no-asm $ ./configure --without-ssl --openssl-fips=dummy Error: --without-ssl is incompatible with --openssl-fips PR-URL: nodejs#12175 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Skip test-cluster-disconnect-handles on Windows. PR-URL: nodejs#12261 Ref: nodejs#12197 (comment) Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- a regular expression that matches the entire error message. PR-URL: nodejs#12139 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Add Alexey Orlenko (@aqrln) to the list of collaborators in README.md PR-URL: nodejs#12273 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rich Trott <rtrott@gmail.com>
Can be cc'ed with `@nodejs/performance`. PR-URL: nodejs#12213 Refs: nodejs#12194 (comment) Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs#12247 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
* Since nodejs#6559 known_issues does run on CI. * Add some notes to explain the expectations around tests in known_issues. PR-URL: nodejs#12262 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#12282 Fixes: nodejs#12280 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Where inclusion of a lengthy URL causes a line to exceed 80 characters in our code base, do not report the line length as a linting error. PR-URL: nodejs#11890 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#12020 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
PR-URL: nodejs#12277 Refs: https://github.com/nodejs/node/blob/master/doc/onboarding.md Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This commit adds lldbinit files from upstream V8 and also adds these so that they get installed when `make install` is run. Original commit message: [tools] add lldbinit The goal of this commit is to add the equivalent to gdbinit but for lldb. I've tried to replicate the commands as close as possible but I'm unsure about the jss command and hoping to get some feedback on it in addition to the bta command which I'm not sure how/when this could be used. This is probably just inexperience on my part. The lldbinit file can be placed into a directory prefixed with dot (.lldbinit) and the python script is currently expected to be in the same directory. The path to the script can be changed manually if needed as well. NOTRY=true Review-Url: https://codereview.chromium.org/2758373002 Cr-Commit-Position: refs/heads/master@{nodejs#44136} PR-URL: nodejs#12061 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
See the commit log of the reverted commit: it's a semver-minor change that can land in the next minor release. This reverts commit 47cbb88.
1f0da51
to
4976e13
Compare
Rebased, /cc @MylesBorins. Node CI: https://ci.nodejs.org/job/node-test-pull-request/7509/ |
404d1bc
to
99dab5b
Compare
landed in c6060fa |
See the commit log of the reverted commit: it's a semver-minor change
that can land in the next minor release.
This reverts commit 47cbb88.
CI: https://ci.nodejs.org/job/node-test-pull-request/6657/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/591/