Skip to content

src: move version metadata into node_metadata{.h, .cc}#24774

Closed
joyeecheung wants to merge 3 commits intonodejs:masterfrom
joyeecheung:move-versions
Closed

src: move version metadata into node_metadata{.h, .cc}#24774
joyeecheung wants to merge 3 commits intonodejs:masterfrom
joyeecheung:move-versions

Conversation

@joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Dec 1, 2018

src: move READONLY_* macros into util.h

Move these macros to util.h so they can be shared among different
C++ files.
Also, renames READONLY_BOOLEAN_PROPERTY to READONLY_TRUE_PROPERTY
(since it sets the property to true), and use ToV8Value in
READONLY_STRING_PROPERTY.

src: move version metadata into node_metadata{.h, .cc}

This patch moves the computation of version metadata from node.cc
into node_metadata{.h, .cc}, and creates a macro that can be
used to iterate over the available version keys (v8, uv, .etc).
This makes the code clearer as now we no longer need to add
all the headers in node.cc just to compute the versions, and
makes it easier to reuse the version definitions.

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

@joyeecheung
src: move READONLY_* macros into util.h
af8a225 
Move these macros to util.h so they can be shared among different
C++ files.
Also, renames `READONLY_BOOLEAN_PROPERTY` to `READONLY_TRUE_PROPERTY`
(since it sets the property to true), and use `ToV8Value` in
`READONLY_STRING_PROPERTY`.
@joyeecheung
src: move version metadata into node_metadata{.h, .cc}
1df332a 
This patch moves the computation of version metadata from node.cc
into node_metadata{.h, .cc}, and creates a macro that can be
used to iterate over the available version keys (v8, uv, .etc).
This makes the code clearer as now we no longer need to add
all the headers in node.cc just to compute the versions, and
makes it easier to reuse the version definitions.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 1, 2018

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1766/pipeline

@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 related to general changes in the lib or src directory. labels Dec 1, 2018
@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 1, 2018

cc @addaleax @jasnell
CI: https://ci.nodejs.org/job/node-test-pull-request/19123/

richardlau
richardlau approved these changes Dec 1, 2018
View reviewed changes
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Nice!

joyeecheung
joyeecheung commented Dec 1, 2018
View reviewed changes
node.gyp Outdated
'src/node_url.cc',
'src/node_util.cc',
'src/node_v8.cc',
'src/node_metadata.cc',
Copy link
Member Author

@joyeecheung joyeecheung Dec 1, 2018

Choose a reason for hiding this comment

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

Oops, should've sort these

@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 2, 2018 •
edited
Loading

@mmarchini BTW, it should be possible to make these directly available as post-mortem metadata that can be consumed by llnode, same goes for the other recent additions like node_binding.cc and node_native_module.cc, maybe we should design some common interface/macros to do this..

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 3, 2018
@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 3, 2018

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

@joyeecheung
Copy link
Member Author

joyeecheung commented Dec 3, 2018

Resume: https://ci.nodejs.org/job/node-test-pull-request/19167/

@Trott
Copy link
Member

Trott commented Dec 4, 2018

Landed in ff7d053...d17d7bd

@Trott Trott closed this Dec 4, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2018
@joyeecheung @Trott
src: move READONLY_* macros into util.h
7761881 
Move these macros to util.h so they can be shared among different
C++ files.
Also, renames `READONLY_BOOLEAN_PROPERTY` to `READONLY_TRUE_PROPERTY`
(since it sets the property to true), and use `ToV8Value` in
`READONLY_STRING_PROPERTY`.

PR-URL: nodejs#24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2018
@joyeecheung @Trott
src: move version metadata into node_metadata{.h, .cc}
d17d7bd 
This patch moves the computation of version metadata from node.cc
into node_metadata{.h, .cc}, and creates a macro that can be
used to iterate over the available version keys (v8, uv, .etc).
This makes the code clearer as now we no longer need to add
all the headers in node.cc just to compute the versions, and
makes it easier to reuse the version definitions.

PR-URL: nodejs#24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@joyeecheung joyeecheung mentioned this pull request Dec 5, 2018
Closed
BridgeAR pushed a commit that referenced this pull request Dec 5, 2018
@joyeecheung @BridgeAR
src: move READONLY_* macros into util.h
53b59b4 
Move these macros to util.h so they can be shared among different
C++ files.
Also, renames `READONLY_BOOLEAN_PROPERTY` to `READONLY_TRUE_PROPERTY`
(since it sets the property to true), and use `ToV8Value` in
`READONLY_STRING_PROPERTY`.

PR-URL: #24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BridgeAR pushed a commit that referenced this pull request Dec 5, 2018
@joyeecheung @BridgeAR
src: move version metadata into node_metadata{.h, .cc}
7c70b61 
This patch moves the computation of version metadata from node.cc
into node_metadata{.h, .cc}, and creates a macro that can be
used to iterate over the available version keys (v8, uv, .etc).
This makes the code clearer as now we no longer need to add
all the headers in node.cc just to compute the versions, and
makes it easier to reuse the version definitions.

PR-URL: #24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 2018
Closed
Closed
Closed
Closed
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@joyeecheung @refack
src: move READONLY_* macros into util.h
f33de1f 
Move these macros to util.h so they can be shared among different
C++ files.
Also, renames `READONLY_BOOLEAN_PROPERTY` to `READONLY_TRUE_PROPERTY`
(since it sets the property to true), and use `ToV8Value` in
`READONLY_STRING_PROPERTY`.

PR-URL: nodejs#24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@joyeecheung @refack
src: move version metadata into node_metadata{.h, .cc}
c5bc716 
This patch moves the computation of version metadata from node.cc
into node_metadata{.h, .cc}, and creates a macro that can be
used to iterate over the available version keys (v8, uv, .etc).
This makes the code clearer as now we no longer need to add
all the headers in node.cc just to compute the versions, and
makes it easier to reuse the version definitions.

PR-URL: nodejs#24774
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request May 13, 2019
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

@richardlau richardlau richardlau 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. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@joyeecheung @nodejs-github-bot @Trott @jasnell @cjihrig @richardlau @BethGriggs