Skip to content

src: remove usages of GetBackingStore in node-api - #44075

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
kvakil:nodeapi-backingstores
Aug 3, 2022
Merged

src: remove usages of GetBackingStore in node-api#44075
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
kvakil:nodeapi-backingstores

Conversation

@kvakil

@kvakilkvakil commented Aug 1, 2022

Copy link
Copy Markdown
Contributor

This removes all usages of GetBackingStore in node-api. See the linked
issue for an explanation.

Refs: #32226
Refs: #43921

@kvakil
src: remove usages of GetBackingStore in node-api
769d815
This removes all usages of GetBackingStore in `node-api`. See the linked
issue for an explanation.
Refs: nodejs#32226
Refs: nodejs#43921
@nodejs-github-bot

nodejs-github-bot commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/node-api

@nodejs-github-botnodejs-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 Aug 1, 2022
@kvakilkvakil mentioned this pull request Aug 1, 2022
Merged
@F3n67uF3n67u added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 1, 2022
mcollina
mcollina approved these changes Aug 1, 2022
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@RaisinTen

RaisinTen commented Aug 1, 2022

Copy link
Copy Markdown
Member

@F3n67u a Jenkins CI run is required for this PR to be marked as author readyPRs that have at least one approval, no pending requests for changes, and a CI started. (the needs-ciPRs that need a full CI run. label denotes that). I'm starting one for this PR, could you take care of the other ones where you applied the label? Thanks!

@nodejs-github-bot

nodejs-github-bot commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45761/

@F3n67u

F3n67u commented Aug 1, 2022

Copy link
Copy Markdown
Contributor

@F3n67u a Jenkins CI run is required for this PR to be marked as author ready PRs that have at least one approval, no pending requests for changes, and a CI started. (the needs-ci PRs that need a full CI run. label denotes that). I'm starting one for this PR, could you take care of the other ones where you applied the label? Thanks!

Done. Thanks for your friendly reminder.

@kvakilkvakil removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 1, 2022
@kvakil

kvakil commented Aug 1, 2022

Copy link
Copy Markdown
ContributorAuthor

(Jenkins seems to have flaked with java.nio.channels.ClosedChannelException. I'm retrying the build.)

@kvakilkvakil added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45776/

@github-actionsgithub-actionsBot mentioned this pull request Aug 2, 2022
Open
11 tasks
@kvakilkvakil added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 2, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45787/

@kvakilkvakil added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 2, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/45792/

@kvakilkvakil added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 2, 2022
@kvakilkvakil self-assigned this Aug 2, 2022
@github-actionsgithub-actionsBot mentioned this pull request Aug 3, 2022
Open
15 tasks
@kvakilkvakil added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 3, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 3, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 4dedd81 into nodejs:mainAug 3, 2022
@nodejs-github-bot

nodejs-github-bot commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator

Landed in 4dedd81

This was referenced Aug 5, 2022
Open
Open
danielleadams pushed a commit that referenced this pull request Aug 16, 2022
@kvakil@danielleadams
src: remove usages of GetBackingStore in node-api
8a79636
This removes all usages of GetBackingStore in `node-api`. See the linked
issue for an explanation.
Refs: #32226
Refs: #43921
PR-URL: #44075
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@danielleadamsdanielleadams mentioned this pull request Aug 16, 2022
Closed
ruyadorno pushed a commit that referenced this pull request Aug 23, 2022
@kvakil@ruyadorno
src: remove usages of GetBackingStore in node-api
8e1b7e2
This removes all usages of GetBackingStore in `node-api`. See the linked
issue for an explanation.
Refs: #32226
Refs: #43921
PR-URL: #44075
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Feng Yu <F3n67u@outlook.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@ruyadornoruyadorno mentioned this pull request Aug 23, 2022
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@gengjiawengengjiawengengjiawen approved these changes
@legendecaslegendecaslegendecas approved these changes
@RaisinTenRaisinTenRaisinTen approved these changes
+1 more reviewer
@F3n67uF3n67uF3n67u approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@kvakilkvakil

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.node-apiIssues and PRs related to the Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@kvakil@nodejs-github-bot@RaisinTen@F3n67u@mcollina@addaleax@gengjiawen@legendecas@targos