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: allow N-API addon in AddLinkedBinding() #35301

Closed
wants to merge 1 commit into from
Closed

src: allow N-API addon in AddLinkedBinding() #35301

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

AddLinkedBinding() can be used to load old-style Node.js addons, but
currently not N-API addons. There’s no good reason not to support
N-API addons as well, so add that.

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

@addaleax
src: allow N-API addon in AddLinkedBinding()
6424abf 
`AddLinkedBinding()` can be used to load old-style Node.js addons, but
currently not N-API addons. There’s no good reason not to support
N-API addons as well, so add that.
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version. node-api Issues and PRs related to the Node-API. embedding Issues and PRs related to embedding Node.js in another project. labels Sep 22, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 22, 2020 •
edited by mhdawson
Loading

Review requested:

  • @nodejs/n-api

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 22, 2020
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 22, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 22, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 22, 2020 •
edited by addaleax
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/33201/ (:green_heart:)

NickNaso
NickNaso approved these changes Sep 23, 2020
View reviewed changes
Copy link
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

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

SGTM

@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. review wanted PRs that need reviews. labels Sep 25, 2020
mhdawson
mhdawson approved these changes Sep 25, 2020
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 25, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@addaleax addaleax added commit-queue Add this label to land a pull request using GitHub Actions. and removed review wanted PRs that need reviews. labels Sep 25, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2020
@github-actions
Copy link
Contributor

Landed in ff38165

@github-actions github-actions bot closed this Sep 26, 2020
nodejs-github-bot pushed a commit that referenced this pull request Sep 26, 2020
@addaleax @nodejs-github-bot
src: allow N-API addon in AddLinkedBinding()
ff38165 
`AddLinkedBinding()` can be used to load old-style Node.js addons, but
currently not N-API addons. There’s no good reason not to support
N-API addons as well, so add that.

PR-URL: #35301
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
MylesBorins pushed a commit that referenced this pull request Sep 29, 2020
@addaleax @MylesBorins
src: allow N-API addon in AddLinkedBinding()
0d8eaa3 
`AddLinkedBinding()` can be used to load old-style Node.js addons, but
currently not N-API addons. There’s no good reason not to support
N-API addons as well, so add that.

PR-URL: #35301
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@MylesBorins MylesBorins mentioned this pull request Sep 29, 2020
Merged
MylesBorins added a commit that referenced this pull request Sep 29, 2020
@MylesBorins
2020-09-29, Version 14.13.0 (Current)
c048607 
Notable changes:

deps:
  * (SEMVER-MINOR) upgrade to libuv 1.40.0 (Colin Ihrig) #35333
module:
  * (SEMVER-MINOR) named exports for CJS via static analysis (Guy Bedford) #35249
  * (SEMVER-MINOR) exports pattern support (Guy Bedford) #34718
src:
  * (SEMVER-MINOR) allow N-API addon in `AddLinkedBinding()` (Anna Henningsen) #35301

PR-URL: #35419
MylesBorins added a commit that referenced this pull request Sep 29, 2020
@MylesBorins
2020-09-29, Version 14.13.0 (Current)
cd884cc 
Notable changes:

deps:
  * (SEMVER-MINOR) upgrade to libuv 1.40.0 (Colin Ihrig) #35333
module:
  * (SEMVER-MINOR) named exports for CJS via static analysis (Guy Bedford) #35249
  * (SEMVER-MINOR) exports pattern support (Guy Bedford) #34718
src:
  * (SEMVER-MINOR) allow N-API addon in `AddLinkedBinding()` (Anna Henningsen) #35301

PR-URL: #35419
@addaleax addaleax deleted the linked-binding-n-api branch October 6, 2020 09:36
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@addaleax @joesepi
src: allow N-API addon in AddLinkedBinding()
07a8787 
`AddLinkedBinding()` can be used to load old-style Node.js addons, but
currently not N-API addons. There’s no good reason not to support
N-API addons as well, so add that.

PR-URL: nodejs#35301
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@MylesBorins @joesepi
2020-09-29, Version 14.13.0 (Current)
ed1a0a0 
Notable changes:

deps:
  * (SEMVER-MINOR) upgrade to libuv 1.40.0 (Colin Ihrig) nodejs#35333
module:
  * (SEMVER-MINOR) named exports for CJS via static analysis (Guy Bedford) nodejs#35249
  * (SEMVER-MINOR) exports pattern support (Guy Bedford) nodejs#34718
src:
  * (SEMVER-MINOR) allow N-API addon in `AddLinkedBinding()` (Anna Henningsen) nodejs#35301

PR-URL: nodejs#35419
@richardlau richardlau mentioned this pull request Dec 14, 2021
Closed
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

@mhdawson mhdawson mhdawson approved these changes

@cjihrig cjihrig cjihrig approved these changes

@NickNaso NickNaso NickNaso 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++. embedding Issues and PRs related to embedding Node.js in another project. lib / src Issues and PRs related to general changes in the lib or src directory. node-api Issues and PRs related to the Node-API. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@addaleax @nodejs-github-bot @NickNaso @cjihrig @mhdawson @himself65