FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

util: use private symbols in JS land directly by joyeecheung · Pull Request #45379 · nodejs/node · GitHub

/ node Public

util: use private symbols in JS land directly - #45379

Closed
joyeecheung wants to merge 2 commits into
nodejs:mainfrom
joyeecheung:hidden-value
Closed

util: use private symbols in JS land directly#45379
joyeecheung wants to merge 2 commits into
nodejs:mainfrom
joyeecheung:hidden-value

Conversation

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

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
Comment thread test/parallel/test-util-internal.js Outdated

Copy link
Copy Markdown
Collaborator

aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 9, 2022

aduh95 commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

This needs a rebase.

aduh95 removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 10, 2022
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.

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member Author

Rebased. PTAL again, thanks! @aduh95 @legendecas

aduh95 commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

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

legendecas 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Still LGTM.

Copy link
Copy Markdown
Collaborator

aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 15, 2022

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member Author

Landed in 066e77a

joyeecheung added a commit that referenced this pull request Nov 17, 2022
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>

Copy link
Copy Markdown
Member

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

daeyeon pushed a commit to daeyeon/node that referenced this pull request Nov 29, 2022
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>
targos pushed a commit that referenced this pull request Dec 12, 2022
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 pushed a commit that referenced this pull request Dec 13, 2022
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
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
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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, 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

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL