Skip to content

doc: add inline comment hints on finalizer signatures#1738

Open
legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:finalizer
Open

doc: add inline comment hints on finalizer signatures#1738
legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:finalizer

Conversation

@legendecas

Copy link
Copy Markdown
Member

Add inline comment hints on finalizer signatures.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.69%. Comparing base (13c28f6) to head (848723c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1738   +/-   ##
=======================================
  Coverage   63.69%   63.69%           
=======================================
  Files           3        3           
  Lines        2063     2063           
  Branches      730      730           
=======================================
  Hits         1314     1314           
  Misses        162      162           
  Partials      587      587           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

Comment thread napi.h
#endif // NAPI_VERSION > 8

#ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
// FinalizerType must implement `void operator()(Env env)`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have we considered using C++ Concepts for this instead of these human-only annotations?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(Though clearly this doesn't need to block the PR...)

@legendecas legendecas Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it will be a semver-major to bump minimum c++ version to c++20. Right now node-addon-api still supports c++17 as in node v18/v20

"node": "^18 || ^20 || >= 21"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Need Triage

Development

Successfully merging this pull request may close these issues.

3 participants