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

doc: fix the example for embedders #42671

Merged
merged 1 commit into from
Apr 13, 2022
Merged

doc: fix the example for embedders #42671

merged 1 commit into from
Apr 13, 2022

Conversation

mmomtchev
Copy link
Contributor

@mmomtchev mmomtchev commented Apr 9, 2022 •
edited by aduh95
Loading

fix the example for embedding Node.js

Fixes: #42670

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. embedding Issues and PRs related to embedding Node.js in another project. labels Apr 9, 2022
@tniessen tniessen requested a review from addaleax April 9, 2022 14:45
RaisinTen
RaisinTen approved these changes Apr 9, 2022
View reviewed changes
Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

As mentioned in

node/doc/api/embedding.md

Line 24 in bb762c5

The full code can be found [in the Node.js source tree][embedtest.cc].
, it's supposed to be the same as https://github.com/nodejs/node/blob/HEAD/test/embedding/embedtest.cc which already has the HandleScope -

node/test/embedding/embedtest.cc

Line 67 in bb762c5

HandleScope handle_scope(isolate);
, so LGTM. Also, why not Refs -> Fixes in the description?

@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 9, 2022
@tniessen tniessen added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 13, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 13, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/42671
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
    at consumeBody (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Response.text (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18)
    at async Request.json (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:49:18)
    at async Request.query (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:107:20)
    at async Request.queryAll (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:134:20)
    at async Request.gql (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:64:22)
    at async PRData.getComments (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21)
    at async Promise.all (index 2)
    at async Promise.all (index 1) {
  type: 'system',
  errno: 'ERR_STREAM_PREMATURE_CLOSE',
  code: 'ERR_STREAM_PREMATURE_CLOSE',
  erroredSysCall: undefined
}
https://github.com/nodejs/node/actions/runs/2161043190

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 13, 2022
@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 13, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 13, 2022
@nodejs-github-bot nodejs-github-bot merged commit d4d7836 into nodejs:master Apr 13, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in d4d7836

@mmomtchev mmomtchev deleted the embed_example branch April 13, 2022 19:33
vmoroz pushed a commit to vmoroz/node that referenced this pull request Apr 13, 2022
@mmomtchev @vmoroz
doc: fix the example for embedders
080b133 
Refs: nodejs#42670

PR-URL: nodejs#42671
Fixes: nodejs#42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Apr 14, 2022
Merged
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
@mmomtchev @xtx1130
doc: fix the example for embedders
4148c34 
Refs: nodejs#42670

PR-URL: nodejs#42671
Fixes: nodejs#42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
juanarbol pushed a commit that referenced this pull request May 31, 2022
@mmomtchev @juanarbol
doc: fix the example for embedders
d4bec32 
Refs: #42670

PR-URL: #42671
Fixes: #42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
@juanarbol juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
@mmomtchev @danielleadams
doc: fix the example for embedders
1e75614 
Refs: #42670

PR-URL: #42671
Fixes: #42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
targos pushed a commit that referenced this pull request Jul 11, 2022
@mmomtchev @targos
doc: fix the example for embedders
4aca853 
Refs: #42670

PR-URL: #42671
Fixes: #42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
@mmomtchev @targos
doc: fix the example for embedders
e566e5c 
Refs: #42670

PR-URL: #42671
Fixes: #42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@mmomtchev @guangwong
doc: fix the example for embedders
74713d6 
Refs: #42670

PR-URL: nodejs/node#42671
Fixes: nodejs/node#42670
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mesteery Mesteery Mesteery approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

@marsonya marsonya marsonya approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax Awaiting requested review from addaleax

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. embedding Issues and PRs related to embedding Node.js in another project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The embedded Node.js example leads to a crash
8 participants
@mmomtchev @nodejs-github-bot @cjihrig @tniessen @marsonya @RaisinTen @Mesteery @jasnell