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.

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

perf_hooks: remove not exist entries from inspect #29528

Closed
wants to merge 2 commits into from
Closed

perf_hooks: remove not exist entries from inspect #29528

wants to merge 2 commits into from

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Sep 11, 2019

Some of the milestones was removed in PR #21247 but entries not removed
from function for inspect PerformanceNodeTiming.

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

@fanatid
perf_hooks: remove not exist entries from inspect
efbf6bd 
Some of the milestones was removed in PR nodejs#21247 but entries not removed
from function for inspect PerformanceNodeTiming.
@nodejs-github-bot nodejs-github-bot added the perf_hooks Issues and PRs related to the implementation of the Performance Timing API. label Sep 11, 2019
@trivikr
Copy link
Member

trivikr commented Sep 11, 2019

@jasnell Should these be removed from test-trace-events-bootstrap.js too?

node/test/parallel/test-trace-events-bootstrap.js

Lines 16 to 23 in 23f8b7d

'thirdPartyMainStart',
'thirdPartyMainEnd',
'clusterSetupStart',
'clusterSetupEnd',
'moduleLoadStart',
'moduleLoadEnd',
'preloadModulesLoadStart',
'preloadModulesLoadEnd'

@jasnell
Copy link
Member

jasnell commented Sep 11, 2019

Yeah they should

@trivikr
Copy link
Member

trivikr commented Sep 11, 2019

Yeah they should

@fanatid would you also like to remove following entries in this PR?

node/test/parallel/test-trace-events-bootstrap.js

Lines 16 to 23 in 23f8b7d

'thirdPartyMainStart',
'thirdPartyMainEnd',
'clusterSetupStart',
'clusterSetupEnd',
'moduleLoadStart',
'moduleLoadEnd',
'preloadModulesLoadStart',
'preloadModulesLoadEnd'

@fanatid
Copy link
Contributor Author

fanatid commented Sep 12, 2019

Removed.

@trivikr
Copy link
Member

trivikr commented Sep 12, 2019

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

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2019
@nodejs-github-bot
Copy link
Collaborator

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

@Trott
Copy link
Member

Trott commented Sep 13, 2019

Landed in 39eb894

@Trott Trott closed this Sep 13, 2019
Trott pushed a commit that referenced this pull request Sep 13, 2019
@fanatid @Trott
perf_hooks: remove non-existent entries from inspect
39eb894 
Some of the milestones was removed in PR #21247 but entries not removed
from function for inspect PerformanceNodeTiming.

PR-URL: #29528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@fanatid fanatid deleted the perf_hooks-remove-node-inspect branch September 13, 2019 20:54
targos pushed a commit that referenced this pull request Sep 20, 2019
@fanatid @targos
perf_hooks: remove non-existent entries from inspect
8823a70 
Some of the milestones was removed in PR #21247 but entries not removed
from function for inspect PerformanceNodeTiming.

PR-URL: #29528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Merged
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
@fanatid @BridgeAR
perf_hooks: remove non-existent entries from inspect
6b30802 
Some of the milestones was removed in PR #21247 but entries not removed
from function for inspect PerformanceNodeTiming.

PR-URL: #29528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Sep 25, 2019
Closed
Closed
Closed
Closed
Closed
Closed
@Semigradsky Semigradsky mentioned this pull request Sep 11, 2020
Merged
7 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

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

@trivikr trivikr trivikr 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. perf_hooks Issues and PRs related to the implementation of the Performance Timing API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@fanatid @trivikr @jasnell @nodejs-github-bot @Trott @addaleax @cjihrig