Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

util: use private symbols in JS land directly #45379

Closed
wants to merge 2 commits into from
Closed

util: use private symbols in JS land directly #45379

wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

Instead of calling into C++ to use the private symbols, use an ObjectTemplate to create an object that holds the symbols and use them directly from JS land.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Nov 8, 2022
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 9, 2022
@github-actions github-actions bot mentioned this pull request Nov 10, 2022
Open
20 tasks
@aduh95
Copy link
Contributor

aduh95 commented Nov 10, 2022

This needs a rebase.

@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 10, 2022
@joyeecheung
util: use private symbols in JS land directly
2ac9634 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.
@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung
Copy link
Member Author

Rebased. PTAL again, thanks! @aduh95 @legendecas

@aduh95
Copy link
Contributor

aduh95 commented Nov 10, 2022

There's a C++ linter failure that needs to be addressed.

This was referenced Nov 11, 2022
Open
Open
@github-actions github-actions bot mentioned this pull request Nov 13, 2022
Open
25 tasks
legendecas
legendecas approved these changes Nov 13, 2022
View reviewed changes
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

Still LGTM.

This was referenced Nov 14, 2022
Open
Open
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 15, 2022
@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung
Copy link
Member Author

Landed in 066e77a

joyeecheung added a commit that referenced this pull request Nov 17, 2022
@joyeecheung
util: use private symbols in JS land directly
066e77a 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ruyadorno
Copy link
Member

This commit does not land cleanly on v19.x-staging.

marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Nov 23, 2022
@joyeecheung @marco-ippolito
util: use private symbols in JS land directly
2f44702 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: nodejs#45379
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
daeyeon pushed a commit to daeyeon/node that referenced this pull request Nov 29, 2022
@joyeecheung @daeyeon
util: use private symbols in JS land directly
18d9474 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: nodejs#45379
Backport-PR-URL: nodejs#45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
@daeyeon daeyeon mentioned this pull request Nov 29, 2022
Closed
targos pushed a commit that referenced this pull request Dec 12, 2022
@joyeecheung @targos
util: use private symbols in JS land directly
8164a41 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
@targos targos mentioned this pull request Dec 12, 2022
Merged
targos pushed a commit that referenced this pull request Dec 13, 2022
@joyeecheung @targos
util: use private symbols in JS land directly
470384e 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@joyeecheung @danielleadams
util: use private symbols in JS land directly
aef2af3 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@joyeecheung @danielleadams
util: use private symbols in JS land directly
3d2bef5 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@joyeecheung @danielleadams
util: use private symbols in JS land directly
3f18a83 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@joyeecheung @danielleadams
util: use private symbols in JS land directly
759e747 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
@joyeecheung @danielleadams
util: use private symbols in JS land directly
2bd79a3 
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@legendecas legendecas legendecas approved these changes

@aduh95 aduh95 aduh95 approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@joyeecheung @nodejs-github-bot @aduh95 @ruyadorno @jasnell @legendecas @targos @daeyeon