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.

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

src: remove calls to SetWrapperClassId() #22975

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Sep 20, 2018

We have migrated from the deprecated RetainedObjectInfo API to
the new EmbedderGraph API, so there is no need to take care
of wrapper class ids anymore since they are dedicated to the
deprecated API (the new API uses a graph insted of ids to retrieve
info about nodes).

Refs: #21741

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

We have migrated from the deprecated RetainedObjectInfo API to
the new EmbedderGraph API, so there is no need to take care
of wrapper class ids anymore since they are dedicated to the
deprecated API (the new API uses a graph insted of ids to retrieve
info about nodes).
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Sep 20, 2018
@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

src/async_wrap.cc Show resolved Hide resolved
@joyeecheung
Copy link
Member Author

Removed the now-also-unused NODE_ASYNC_ID_OFFSET and BUFFER_ID (thanks to suggestion from @addaleax).
CI: https://ci.nodejs.org/job/node-test-pull-request/17346/

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 20, 2018
@danbev
Copy link
Contributor

danbev commented Sep 21, 2018

Nit: Minor typo in the commit message (insted):

the new API uses a graph insted of ids to retrieve info about nodes)

@joyeecheung
Copy link
Member Author

Landed in c67d3d0, thanks!

joyeecheung added a commit that referenced this pull request Sep 23, 2018
We have migrated from the deprecated RetainedObjectInfo API to
the new EmbedderGraph API, so there is no need to take care
of wrapper class ids anymore since they are dedicated to the
deprecated API (the new API uses a graph instead of ids to retrieve
info about nodes).

PR-URL: #22975
Refs: #21741
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@danbev danbev closed this Sep 24, 2018
targos pushed a commit that referenced this pull request Sep 24, 2018
We have migrated from the deprecated RetainedObjectInfo API to
the new EmbedderGraph API, so there is no need to take care
of wrapper class ids anymore since they are dedicated to the
deprecated API (the new API uses a graph instead of ids to retrieve
info about nodes).

PR-URL: #22975
Refs: #21741
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants