Skip to content

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

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
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

@addaleax addaleax commented Sep 22, 2020

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

nodejs-github-bot commented Sep 22, 2020

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

nodejs-github-bot commented Sep 25, 2020

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

github-actions bot commented Sep 26, 2020

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

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.

6 participants

@addaleax @nodejs-github-bot @NickNaso @cjihrig @mhdawson @himself65