GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: refactor binding data for deserialization support by joyeecheung · Pull Request #37112 · nodejs/node

Conversation

@joyeecheung

@joyeecheung joyeecheung commented Jan 28, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Refs: #36943
Refs: #35711

src: rename binding_data_name to type_name in BindingData

Previously, this was a per-class string constant for BindingData
which is used as keys for identifying these objects in the binding
data map. These are just type names of the BindingData.
This patch renames the variable to type_name so that
we can generalize this constant for other BaseObjects and use
it for debugging and logging the types of other BaseObjects.

src: refactor v8 binding

  1. Put the v8 binding data class into a header so we can reuse
    the class definition during deserialization.
  2. Put the v8 binding code into node::v8_utils namespace for
    clarity.
  3. Move the binding data property initialization into its
    constructor so that we can reuse it during deserialization
  4. Reorder the v8 binding initialization so that we don't
    unnecessarily initialize the properties in a loop

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. labels Jan 28, 2021
@joyeecheung
joyeecheung force-pushed the refactor-binding branch from b1971a1 to 3f1916e Compare January 28, 2021 16:39
@joyeecheung joyeecheung mentioned this pull request Jan 28, 2021
bootstrap: include v8 and fs modules in the startup snapshot #36943
Closed
@nodejs-github-bot

nodejs-github-bot commented Jan 28, 2021

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

juanarbol
juanarbol approved these changes Jan 29, 2021
View reviewed changes
joyeecheung added 2 commits February 5, 2021 20:00
@joyeecheung
src: rename binding_data_name to type_name in BindingData
e35ea37
Previously, this was a per-class string constant for BindingData
which is used as keys for identifying these objects in the binding
data map. These are just type names of the BindingData.
This patch renames the variable to type_name so that
we can generalize this constant for other BaseObjects and use
it for debugging and logging the types of other BaseObjects.
@joyeecheung
src: refactor v8 binding
910040d
1. Put the v8 binding data class into a header so we can reuse
  the class definition during deserialization.
2. Put the v8 binding code into node::v8_utils namespace for
  clarity.
3. Move the binding data property initialization into its
  constructor so that we can reuse it during deserialization
4. Reorder the v8 binding initialization so that we don't
  unnecessarily initialize the properties in a loop
@joyeecheung
joyeecheung force-pushed the refactor-binding branch from 3f1916e to 910040d Compare February 5, 2021 12:09
@joyeecheung

joyeecheung commented Feb 5, 2021

Copy link
Copy Markdown
Member Author

Rebased after quic was removed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Feb 5, 2021

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Feb 8, 2021

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joyeecheung

joyeecheung commented Feb 8, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Landed in b346cd1...7e6bb66

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joyeecheung joyeecheung closed this Feb 8, 2021
joyeecheung added a commit that referenced this pull request Feb 8, 2021
@joyeecheung
src: rename binding_data_name to type_name in BindingData
1b7531c
Previously, this was a per-class string constant for BindingData
which is used as keys for identifying these objects in the binding
data map. These are just type names of the BindingData.
This patch renames the variable to type_name so that
we can generalize this constant for other BaseObjects and use
it for debugging and logging the types of other BaseObjects.

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
joyeecheung added a commit that referenced this pull request Feb 8, 2021
@joyeecheung
src: refactor v8 binding
7e6bb66
1. Put the v8 binding data class into a header so we can reuse
  the class definition during deserialization.
2. Put the v8 binding code into node::v8_utils namespace for
  clarity.
3. Move the binding data property initialization into its
  constructor so that we can reuse it during deserialization
4. Reorder the v8 binding initialization so that we don't
  unnecessarily initialize the properties in a loop

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
@joyeecheung @danielleadams
src: rename binding_data_name to type_name in BindingData
54d36b0
Previously, this was a per-class string constant for BindingData
which is used as keys for identifying these objects in the binding
data map. These are just type names of the BindingData.
This patch renames the variable to type_name so that
we can generalize this constant for other BaseObjects and use
it for debugging and logging the types of other BaseObjects.

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
@joyeecheung @danielleadams
src: refactor v8 binding
1a9bcdf
1. Put the v8 binding data class into a header so we can reuse
  the class definition during deserialization.
2. Put the v8 binding code into node::v8_utils namespace for
  clarity.
3. Move the binding data property initialization into its
  constructor so that we can reuse it during deserialization
4. Reorder the v8 binding initialization so that we don't
  unnecessarily initialize the properties in a loop

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
This was referenced Feb 16, 2021
v15.9.0 proposal #37405
Closed
v15.9.0 proposal #37406
Merged
targos pushed a commit that referenced this pull request May 1, 2021
@joyeecheung @targos
src: rename binding_data_name to type_name in BindingData
33ebf5d
Previously, this was a per-class string constant for BindingData
which is used as keys for identifying these objects in the binding
data map. These are just type names of the BindingData.
This patch renames the variable to type_name so that
we can generalize this constant for other BaseObjects and use
it for debugging and logging the types of other BaseObjects.

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
@joyeecheung @targos
src: refactor v8 binding
f476c6d
1. Put the v8 binding data class into a header so we can reuse
  the class definition during deserialization.
2. Put the v8 binding code into node::v8_utils namespace for
  clarity.
3. Move the binding data property initialization into its
  constructor so that we can reuse it during deserialization
4. Reorder the v8 binding initialization so that we don't
  unnecessarily initialize the properties in a loop

PR-URL: #37112
Refs: #36943
Reviewed-By: Juan José Arboleda <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
v14.17.0 proposal #38507
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juanarbol juanarbol juanarbol approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@joyeecheung @nodejs-github-bot @juanarbol
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.