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: fix loop delay resolution validation #38166

Closed
wants to merge 3 commits into from
Closed

perf_hooks: fix loop delay resolution validation #38166

wants to merge 3 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 9, 2021

Fixes: #38160

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 9, 2021
addaleax
addaleax approved these changes Apr 9, 2021
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

This seems fine, but why not just widen the type of interval on the C++ type to uint64_t?

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 9, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

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

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

Flarna pushed a commit that referenced this pull request Apr 11, 2021
@aduh95 @Flarna
perf_hooks: fix loop delay resolution validation
548cbf0 
Fixes: #38160

PR-URL: #38166
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@Flarna
Copy link
Member

Flarna commented Apr 11, 2021

Landed in 548cbf0

@Flarna Flarna closed this Apr 11, 2021
@aduh95 aduh95 deleted the fix-perf_hooks-abort branch April 11, 2021 21:27
@BethGriggs BethGriggs mentioned this pull request Apr 12, 2021
Merged
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

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@Flarna Flarna Flarna 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. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

perf_hooks.monitorEventLoopDelay results in an abort
7 participants
@aduh95 @nodejs-github-bot @Flarna @jasnell @addaleax @lpinca @cjihrig