Skip to content

src,lib: group properties used as constants from util binding#45539

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 3 commits into from
Nov 24, 2022
Merged

src,lib: group properties used as constants from util binding #45539

nodejs-github-bot merged 3 commits into from
Nov 24, 2022

Conversation

@daeyeon
Copy link
Member

@daeyeondaeyeon commented Nov 20, 2022•
edited
Loading

The ways to create properties used as constants are scattered in util internal binding.
This PR gets them created in one code block and creates an object holding all of them.

Similar object patterns can be found in other bindings.

node/lib/dgram.js

Lines 71 to 75 in db88483

const{
constants: {UV_UDP_IPV6ONLY},
UDP,
SendWrap
}=internalBinding('udp_wrap');

node/lib/internal/perf/nodetiming.js

Lines 21 to 31 in db88483

const{
constants: {
NODE_PERFORMANCE_MILESTONE_NODE_START,
NODE_PERFORMANCE_MILESTONE_V8_START,
NODE_PERFORMANCE_MILESTONE_LOOP_START,
NODE_PERFORMANCE_MILESTONE_LOOP_EXIT,
NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE,
NODE_PERFORMANCE_MILESTONE_ENVIRONMENT
},
loopIdleTime,
}=internalBinding('performance');

Signed-off-by: Daeyeon Jeong [email protected]

@daeyeon
src,lib: group properties used as constants from util binding
8b95729
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 20, 2022•
edited by daeyeon
Loading

Review requested:

  • @nodejs/startup
  • @nodejs/cpp-reviewers

@nodejs-github-botnodejs-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 20, 2022
@daeyeondaeyeon added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 20, 2022
@daeyeondaeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
joyeecheung
joyeecheung reviewed Nov 22, 2022
View reviewed changes
@daeyeondaeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@daeyeondaeyeonforce-pushed the main.util-constants-221120.Sun.a9c7 branch from e1cc165 to 397662eCompareNovember 23, 2022 06:40
@daeyeondaeyeon added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 23, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 23, 2022

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

@github-actionsgithub-actionsbot mentioned this pull request Nov 24, 2022
Open
16 tasks
@daeyeondaeyeon added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 24, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 24, 2022
@nodejs-github-botnodejs-github-bot merged commit be9cd3e into nodejs:mainNov 24, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 24, 2022

Landed in be9cd3e

@daeyeondaeyeon deleted the main.util-constants-221120.Sun.a9c7 branch November 24, 2022 02:56
@ruyadorno
Copy link
Member

ruyadorno commented Nov 24, 2022

@daeyeonbe9cd3e does not land cleanly on the release branch v19.x-staging. It will need backporting.

marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Nov 24, 2022
@daeyeon@marco-ippolito
src,lib: group properties used as constants from util binding
1e0aac3
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: nodejs#45539 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
This was referenced Nov 25, 2022
Open
Open
Open
Open
daeyeon added a commit to daeyeon/node that referenced this pull request Nov 29, 2022
@daeyeon
src,lib: group properties used as constants from util binding
0567596
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: nodejs#45539 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@daeyeondaeyeon mentioned this pull request Nov 29, 2022
Closed
daeyeon added a commit to daeyeon/node that referenced this pull request Nov 29, 2022
@daeyeon
src,lib: group properties used as constants from util binding
f06e8e9
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: nodejs#45539 Backport-PR-URL: nodejs#45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Dec 12, 2022
@daeyeon@targos
src,lib: group properties used as constants from util binding
b401800
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@targostargos mentioned this pull request Dec 12, 2022
Merged
targos pushed a commit that referenced this pull request Dec 13, 2022
@daeyeon@targos
src,lib: group properties used as constants from util binding
2fbe2f9
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@daeyeon@danielleadams
src,lib: group properties used as constants from util binding
e770c80
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@daeyeon@danielleadams
src,lib: group properties used as constants from util binding
2887220
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@daeyeon@danielleadams
src,lib: group properties used as constants from util binding
90e8418
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@daeyeon@danielleadams
src,lib: group properties used as constants from util binding
714ca6d
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
@daeyeon@danielleadams
src,lib: group properties used as constants from util binding
b02b3c8
Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: #45539 Backport-PR-URL: #45674 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@harrisiharrisi mentioned this pull request Feb 19, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joyeecheungjoyeecheungjoyeecheung approved these changes

@JungMinuJungMinuJungMinu approved these changes

@legendecaslegendecaslegendecas approved these changes

@RaisinTenRaisinTenRaisinTen approved these changes

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@daeyeon@nodejs-github-bot@ruyadorno@joyeecheung@JungMinu@legendecas@RaisinTen@targos