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.

Sign up for GitHub

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

Jump to bottom

src: use MaybeLocal.ToLocal instead of IsEmpty worker #33599

Closed
wants to merge 1 commit into from
Closed

src: use MaybeLocal.ToLocal instead of IsEmpty worker #33599

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 28, 2020

This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@danbev
src: use MaybeLocal.ToLocal instead of IsEmpty worker
d70214d 
This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. worker Issues and PRs related to Worker support. labels May 28, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31593/

@danbev
Copy link
Contributor Author

danbev commented May 28, 2020

Re-run of failing node-test-commit-aix ✔️

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 28, 2020
@targos targos self-assigned this May 30, 2020
targos pushed a commit that referenced this pull request May 30, 2020
@danbev @targos
src: use MaybeLocal.ToLocal instead of IsEmpty worker
c4cced0 
This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.

PR-URL: #33599
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@targos
Copy link
Member

targos commented May 30, 2020

Landed in c4cced0

@targos targos closed this May 30, 2020
codebytere pushed a commit that referenced this pull request Jun 18, 2020
@danbev @codebytere
src: use MaybeLocal.ToLocal instead of IsEmpty worker
6ef2efe 
This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.

PR-URL: #33599
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@codebytere codebytere mentioned this pull request Jun 28, 2020
Merged
codebytere pushed a commit that referenced this pull request Jun 30, 2020
@danbev @codebytere
src: use MaybeLocal.ToLocal instead of IsEmpty worker
8d4180f 
This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.

PR-URL: #33599
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
codebytere pushed a commit that referenced this pull request Jul 9, 2020
@danbev @codebytere
src: use MaybeLocal.ToLocal instead of IsEmpty worker
99371ad 
This commit suggest using MaybeLocal.ToLocal and passing in the
Local<String> node_opts. It also introduces a variable, arg_v8, of type
Local<String> to enable the use of ToLocal.

The motivation for doing this is that the following
MaybeLocal::ToLocalChecked and MaybeLocal::FromMaybe calls can then be
avoided.

PR-URL: #33599
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@codebytere codebytere mentioned this pull request Jul 13, 2020
Merged
@targos targos removed their assignment Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@himself65 himself65 himself65 approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. worker Issues and PRs related to Worker support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@danbev @nodejs-github-bot @targos @jasnell @addaleax @cjihrig @himself65 @BridgeAR