-
Notifications
You must be signed in to change notification settings - Fork 31.3k
doc: clarify behaviour of node-api adjust function #57463
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
Conversation
Refs: nodejs#57351 - based on recent request to update one of the tests Signed-off-by: Michael Dawson <midawson@redhat.com>
Review requested:
|
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Michael Dawson <midawson@redhat.com>
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
@legendecas accepted your suggestions and then tweaked again. I am still wondering if we should include the comment about calling with -512 if you called with +512 before. If its ok to call +512 twice and then -1024 later I'm not sure we want to include that. |
doc/api/n-api.md
Outdated
often than it would otherwise. | ||
|
||
This function is expected to be invoked symmetrically. If it is invoked with +512KB, | ||
it is expected to be invoked -512KB in sum at a later time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still wondering if we should include the comment about calling with -512 if you called with +512 before. If its ok to call +512 twice and then -1024 later I'm not sure we want to include that.
"in sum" should address the concern? I think this sentence now is saying the value should be in symmetry.
Signed-off-by: Michael Dawson <midawson@redhat.com>
Commit Queue failed- Loading data for nodejs/node/pull/57463 ✔ Done loading data for nodejs/node/pull/57463 ----------------------------------- PR info ------------------------------------ Title doc: clarify behaviour of node-api adjust function (#57463) Author Michael Dawson <midawson@redhat.com> (@mhdawson) Branch mhdawson:node-api-adjust-doc -> nodejs:main Labels doc, node-api, author ready Commits 14 - doc: clarify behaviour of node-api adjust function - Update doc/api/n-api.md - Update doc/api/n-api.md - squash: address comments - Update doc/api/n-api.md - Update doc/api/n-api.md - squash: make linter happy - Update doc/api/n-api.md - Update doc/api/n-api.md - Update doc/api/n-api.md - squash: make linter happy again - Update doc/api/n-api.md - Update doc/api/n-api.md - Update doc/api/n-api.md Committers 2 - Michael Dawson <midawson@redhat.com> - GitHub <noreply@github.com> PR-URL: https://github.com/nodejs/node/pull/57463 Refs: https://github.com/nodejs/node/pull/57351 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/57463 Refs: https://github.com/nodejs/node/pull/57351 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com> -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 14 Mar 2025 16:33:21 GMT ✔ Approvals: 3 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/57463#pullrequestreview-2688093622 ✔ - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/57463#pullrequestreview-2708009128 ✔ - Vladimir Morozov (@vmoroz): https://github.com/nodejs/node/pull/57463#pullrequestreview-2692160164 ✔ Last GitHub CI successful ℹ Green GitHub CI is sufficient -------------------------------------------------------------------------------- ✔ 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 57463 From https://github.com/nodejs/node * branch refs/pull/57463/merge -> FETCH_HEAD ✔ Fetched commits as af75d04a76f1..0792c648255b -------------------------------------------------------------------------------- [main 445d937f04] doc: clarify behaviour of node-api adjust function Author: Michael Dawson <midawson@redhat.com> Date: Fri Mar 14 16:31:39 2025 +0000 1 file changed, 9 insertions(+), 5 deletions(-) [main 309b4a8d5c] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Mon Mar 17 11:05:35 2025 -0400 1 file changed, 1 insertion(+), 1 deletion(-) [main d26bae84e3] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Mon Mar 17 11:07:01 2025 -0400 1 file changed, 1 insertion(+), 1 deletion(-) [main f25b0c9b60] squash: address comments Author: Michael Dawson <mdawson@devrus.com> Date: Mon Mar 17 17:31:08 2025 -0400 1 file changed, 4 insertions(+), 4 deletions(-) [main d37760a389] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Mon Mar 17 17:31:54 2025 -0400 1 file changed, 1 insertion(+), 1 deletion(-) [main a07cb6213b] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Mon Mar 17 17:33:06 2025 -0400 1 file changed, 1 insertion(+), 1 deletion(-) [main 39ace7aa08] squash: make linter happy Author: Michael Dawson <midawson@redhat.com> Date: Mon Mar 17 21:59:59 2025 +0000 1 file changed, 1 insertion(+), 1 deletion(-) [main 9cb313079e] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Thu Mar 20 16:20:28 2025 -0400 1 file changed, 5 insertions(+), 4 deletions(-) [main 40a46af5be] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Thu Mar 20 16:20:40 2025 -0400 1 file changed, 3 insertions(+) [main 8cb12f5c27] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Thu Mar 20 16:24:43 2025 -0400 1 file changed, 3 insertions(+), 3 deletions(-) [main 6dbbbe56d8] squash: make linter happy again Author: Michael Dawson <midawson@redhat.com> Date: Fri Mar 21 13:07:38 2025 +0000 1 file changed, 3 insertions(+), 3 deletions(-) [main 2a1f4efa75] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Fri Mar 21 11:28:34 2025 -0400 1 file changed, 3 insertions(+), 1 deletion(-) [main 92379c52fb] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Fri Mar 21 11:28:57 2025 -0400 1 file changed, 1 deletion(-) [main c81ea08ffb] Update doc/api/n-api.md Author: Michael Dawson <mdawson@devrus.com> Date: Fri Mar 21 11:29:12 2025 -0400 1 file changed, 3 insertions(+), 3 deletions(-) ✔ Patches applied There are 14 commits in the PR. Attempting autorebase. Rebasing (2/28) Executing: git node land --amend --yes ⚠ Found Refs: https://github.com/nodejs/node/pull/57351, skipping.. --------------------------------- New Message ---------------------------------- doc: clarify behaviour of node-api adjust function
Signed-off-by: Michael Dawson <midawson@redhat.com>
|
Landed in ca74d64 |
Refs: nodejs#57351 - based on recent request to update one of the tests Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: nodejs#57463 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Refs: nodejs#57351 - based on recent request to update one of the tests Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: nodejs#57463 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Refs: #57351