-
-
Notifications
You must be signed in to change notification settings - Fork 493
test: remove outdated V8 flag #895
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
Conversation
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. Refs: v8/v8@0a480c3
|
cc @mhdawson could you please take a look once you are free? :) |
| if (typeof global.gc !== 'function') { | ||
| // Construct the correct (version-dependent) command-line args. | ||
| let args = ['--expose-gc', '--no-concurrent-array-buffer-freeing']; | ||
| let args = ['--expose-gc']; |
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.
Did you check the history of why this was added in the first place. I'm wondering if adding it needs to be Node.js version dependent versus just removing completely (ie might results in failures on older Node.js versions...)
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.
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.
I added a check to conditionally add the option if v8 has a major version that is less than 9. PTAL.
mhdawson
left a comment
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, thanks for updating.
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. PR-URL: #895 Refs: v8/v8@0a480c3 Reviewed-By: Michael Dawson <midawson@redhat.com>
|
Landed as f7ed249 thanks @RaisinTen |
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. PR-URL: nodejs/node-addon-api#895 Refs: v8/v8@0a480c3 Reviewed-By: Michael Dawson <midawson@redhat.com>
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. PR-URL: nodejs/node-addon-api#895 Refs: v8/v8@0a480c3 Reviewed-By: Michael Dawson <midawson@redhat.com>
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. PR-URL: nodejs/node-addon-api#895 Refs: v8/v8@0a480c3 Reviewed-By: Michael Dawson <midawson@redhat.com>
The `--concurrent-array-buffer-freeing` flag is going to be removed upstream in V8 9.0. PR-URL: nodejs/node-addon-api#895 Refs: v8/v8@0a480c3 Reviewed-By: Michael Dawson <midawson@redhat.com>
The
--concurrent-array-buffer-freeingflag is going to be removedupstream in V8 9.0.
Refs: v8/v8@0a480c3