GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — n-api: improve consistency of how we get context by mhdawson · Pull Request #36068 · nodejs/node

Conversation

@mhdawson

@mhdawson mhdawson commented Nov 10, 2020

Copy link
Copy Markdown
Member

Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson [email protected]

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@mhdawson
n-api: improve consistency of how we get context
9f559af 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>
@nodejs-github-bot

nodejs-github-bot commented Nov 10, 2020 •
edited by mhdawson
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/n-api
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 10, 2020
juanarbol
juanarbol approved these changes Nov 10, 2020
View reviewed changes
NickNaso
NickNaso approved these changes Nov 10, 2020
View reviewed changes

@NickNaso NickNaso left a comment

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
gabrielschulhof
gabrielschulhof approved these changes Nov 10, 2020
View reviewed changes
@gabrielschulhof gabrielschulhof added the node-api Issues and PRs related to Node-API. label Nov 10, 2020
benjamingr
benjamingr approved these changes Nov 10, 2020
View reviewed changes
Trott
Trott approved these changes Nov 11, 2020
View reviewed changes
@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 11, 2020
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 11, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Nov 12, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34351/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
legendecas
legendecas approved these changes Nov 13, 2020
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Nov 14, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34393/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Nov 17, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34418/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Nov 17, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/34419/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson

mhdawson commented Nov 17, 2020

Copy link
Copy Markdown
Member Author

CI is green will land.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mhdawson added a commit that referenced this pull request Nov 17, 2020
@mhdawson
n-api: improve consistency of how we get context
cbfa2d1 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36068
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@mhdawson

mhdawson commented Nov 17, 2020

Copy link
Copy Markdown
Member Author

Landed in cbfa2d1
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhdawson mhdawson closed this Nov 17, 2020
codebytere pushed a commit that referenced this pull request Nov 22, 2020
@mhdawson @codebytere
n-api: improve consistency of how we get context
3d623d8 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36068
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@codebytere codebytere mentioned this pull request Nov 22, 2020
v15.3.0 proposal #36232
Merged
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
@mhdawson @BethGriggs
n-api: improve consistency of how we get context
b42949f 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36068
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
@mhdawson @BethGriggs
n-api: improve consistency of how we get context
4b383e1 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36068
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
v14.15.2 proposal #36476
Merged
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
@mhdawson @BethGriggs
n-api: improve consistency of how we get context
dde727e 
Refs: nodejs/node-addon-api#764
Improve the consistency of how we get a context
when needed. We generally used env->context() in N-API
but there were are few exceptions that this PR addresses.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #36068
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes
@gabrielschulhof gabrielschulhof gabrielschulhof approved these changes
@benjamingr benjamingr benjamingr approved these changes
@legendecas legendecas legendecas approved these changes
@juanarbol juanarbol juanarbol approved these changes
+1 more reviewer
@NickNaso NickNaso NickNaso approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@mhdawson @nodejs-github-bot @NickNaso @Trott @gabrielschulhof @benjamingr @legendecas @juanarbol
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.