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

lib: streamline process.binding() handling #50773

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

  • Make processBindingAllowList a separate list from runtimeDeprecatedList and legacyWrapperList instead of being an umbrella one, so it's easier to see the stages the bindings are in.
  • Cache process.binding() results so we don't need to mutate runtimeDeprecatedList.

- Make processBindingAllowList a separate list from
  runtimeDeprecatedList and legacyWrapperList instead of being
  an umbrella one, so it's easier to see the stages the bindings
  are in.
- Cache process.binding() results so we don't need to mutate
  runtimeDeprecatedList.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @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 17, 2023
lib/internal/bootstrap/realm.js Outdated Show resolved Hide resolved
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Nov 28, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 28, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/50773
✔  Done loading data for nodejs/node/pull/50773
----------------------------------- PR info ------------------------------------
Title      lib: streamline process.binding() handling (#50773)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:binding-wrappers -> nodejs:main
Labels     lib / src, needs-ci, commit-queue-squash
Commits    2
 - lib: streamline process.binding() handling
 - fixup! lib: streamline process.binding() handling
Committers 2
 - Joyee Cheung 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/50773
Reviewed-By: Vinícius Lourenço Claro Cardoso 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50773
Reviewed-By: Vinícius Lourenço Claro Cardoso 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - fixup! lib: streamline process.binding() handling
   ℹ  This PR was created on Fri, 17 Nov 2023 18:29:05 GMT
   ✔  Approvals: 2
   ✔  - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/50773#pullrequestreview-1738093565
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/50773#pullrequestreview-1738716129
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-11-25T14:32:09Z: https://ci.nodejs.org/job/node-test-pull-request/55910/
- Querying data for job/node-test-pull-request/55910/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7024794514

joyeecheung added a commit that referenced this pull request Nov 28, 2023
- Make processBindingAllowList a separate list from
  runtimeDeprecatedList and legacyWrapperList instead of being
  an umbrella one, so it's easier to see the stages the bindings
  are in.
- Cache process.binding() results so we don't need to mutate
  runtimeDeprecatedList.

PR-URL: #50773
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@joyeecheung
Copy link
Member Author

Landed in 940910d (with linter fix applied)

targos pushed a commit that referenced this pull request Dec 4, 2023
- Make processBindingAllowList a separate list from
  runtimeDeprecatedList and legacyWrapperList instead of being
  an umbrella one, so it's easier to see the stages the bindings
  are in.
- Cache process.binding() results so we don't need to mutate
  runtimeDeprecatedList.

PR-URL: #50773
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@targos targos mentioned this pull request Dec 4, 2023
richardlau pushed a commit that referenced this pull request Mar 25, 2024
- Make processBindingAllowList a separate list from
  runtimeDeprecatedList and legacyWrapperList instead of being
  an umbrella one, so it's easier to see the stages the bindings
  are in.
- Cache process.binding() results so we don't need to mutate
  runtimeDeprecatedList.

PR-URL: #50773
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

4 participants