Skip to content

node-api: add warning for NAPI_EXPERIMENTAL #58280

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

Merged
merged 2 commits into from
Jun 13, 2025

Conversation

miguelmarcondesf
Copy link
Contributor

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels May 11, 2025
@miguelmarcondesf miguelmarcondesf changed the title n-api: add warning for NAPI_EXPERIMENTAL node-api: add warning for NAPI_EXPERIMENTAL May 11, 2025
@miguelmarcondesf miguelmarcondesf requested a review from vmoroz May 15, 2025 14:12
@miguelmarcondesf miguelmarcondesf marked this pull request as ready for review May 15, 2025 14:13
@mhdawson mhdawson added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 16, 2025
@mhdawson mhdawson moved this from Need Triage to In Progress in Node-API Team Project May 16, 2025
Copy link

codecov bot commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.16%. Comparing base (02a1505) to head (5961ebf).
Report is 61 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58280      +/-   ##
==========================================
- Coverage   90.22%   90.16%   -0.07%     
==========================================
  Files         635      636       +1     
  Lines      187513   187891     +378     
  Branches    36840    36881      +41     
==========================================
+ Hits       169176   169404     +228     
- Misses      11106    11232     +126     
- Partials     7231     7255      +24     

see 65 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

@mhdawson mhdawson moved this from In Progress to Has PR in Node-API Team Project May 30, 2025
@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 6, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 6, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member

CI failed on windows:

00:16:20 C:\workspace\node-compile-windows-debug\node\src\js_native_api.h(27,1): error C1021: invalid preprocessor command 'warning' [C:\workspace\node-compile-windows-debug\node\test\addons\hello-world\build\binding.vcxproj]
00:16:20   (compiling source file '../binding.cc')
00:16:20   
00:16:21   binding2.cc
00:16:21   win_delay_load_hook.cc
00:16:23 C:\workspace\node-compile-windows-debug\node\src\js_native_api.h(27,1): error C1021: invalid preprocessor command 'warning' [C:\workspace\node-compile-windows-debug\node\test\addons\hello-world\build\binding2.vcxproj]
00:16:23   (compiling source file '../binding2.cc')

@nodejs-github-bot
Copy link
Collaborator

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2025
@nodejs-github-bot
Copy link
Collaborator

@miguelmarcondesf
Copy link
Contributor Author

CI failed on windows:

00:16:20 C:\workspace\node-compile-windows-debug\node\src\js_native_api.h(27,1): error C1021: invalid preprocessor command 'warning' [C:\workspace\node-compile-windows-debug\node\test\addons\hello-world\build\binding.vcxproj]
00:16:20   (compiling source file '../binding.cc')
00:16:20   
00:16:21   binding2.cc
00:16:21   win_delay_load_hook.cc
00:16:23 C:\workspace\node-compile-windows-debug\node\src\js_native_api.h(27,1): error C1021: invalid preprocessor command 'warning' [C:\workspace\node-compile-windows-debug\node\test\addons\hello-world\build\binding2.vcxproj]
00:16:23   (compiling source file '../binding2.cc')

Thanks, I think now we'll solve for this case as well

@nodejs-github-bot
Copy link
Collaborator

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

@legendecas legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2025
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 13, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/58280
✔  Done loading data for nodejs/node/pull/58280
----------------------------------- PR info ------------------------------------
Title      node-api: add warning for NAPI_EXPERIMENTAL (#58280)
Author     Miguel Marcondes Filho <miguelmarcondesfilho@gmail.com> (@miguelmarcondesf)
Branch     miguelmarcondesf:napi-461 -> nodejs:main
Labels     c++, semver-major, node-api
Commits    2
 - node-api: add warning for NAPI_EXPERIMENTAL
 - node-api: add platform-specific warning
Committers 1
 - Miguel Marcondes <miguelmarcondesfilho@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58280
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58280
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 11 May 2025 16:41:45 GMT
   ✔  Approvals: 5
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/58280#pullrequestreview-2906296417
   ✔  - Edy Silva (@geeksilva97): https://github.com/nodejs/node/pull/58280#pullrequestreview-2910681798
   ✔  - Stefan Stojanovic (@StefanStojanovic): https://github.com/nodejs/node/pull/58280#pullrequestreview-2913137912
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/58280#pullrequestreview-2925230253
   ✔  - Vladimir Morozov (@vmoroz): https://github.com/nodejs/node/pull/58280#pullrequestreview-2925232473
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-09T21:14:42Z: https://ci.nodejs.org/job/node-test-pull-request/67369/
- Querying data for job/node-test-pull-request/67369/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 58280
From https://github.com/nodejs/node
 * branch                  refs/pull/58280/merge -> FETCH_HEAD
✔  Fetched commits as 3ac0e28a7f19..5961ebfad9e9
--------------------------------------------------------------------------------
[main c882c79ba6] node-api: add warning for NAPI_EXPERIMENTAL
 Author: Miguel Marcondes <miguelmarcondesfilho@gmail.com>
 Date: Sun May 11 13:41:02 2025 -0300
 1 file changed, 6 insertions(+)
[main 09338c6b1d] node-api: add platform-specific warning
 Author: Miguel Marcondes <miguelmarcondesfilho@gmail.com>
 Date: Mon Jun 9 11:35:01 2025 -0300
 1 file changed, 7 insertions(+), 1 deletion(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
node-api: add warning for NAPI_EXPERIMENTAL

PR-URL: #58280
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>

[detached HEAD 72c379984a] node-api: add warning for NAPI_EXPERIMENTAL
Author: Miguel Marcondes <miguelmarcondesfilho@gmail.com>
Date: Sun May 11 13:41:02 2025 -0300
1 file changed, 6 insertions(+)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
node-api: add platform-specific warning

PR-URL: #58280
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>

[detached HEAD d1fa2bc79c] node-api: add platform-specific warning
Author: Miguel Marcondes <miguelmarcondesfilho@gmail.com>
Date: Mon Jun 9 11:35:01 2025 -0300
1 file changed, 7 insertions(+), 1 deletion(-)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/15638257237

@legendecas legendecas added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 13, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2025
@nodejs-github-bot nodejs-github-bot merged commit 5fe7800 into nodejs:main Jun 13, 2025
72 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5fe7800

@github-project-automation github-project-automation bot moved this from Has PR to Done in Node-API Team Project Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. node-api Issues and PRs related to the Node-API. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
Development

Successfully merging this pull request may close these issues.

7 participants