Skip to content

src: remove a stale comment in async_hooks#43317

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
aduh95 merged 1 commit into from
Jun 30, 2022
Merged

src: remove a stale comment in async_hooks#43317

aduh95 merged 1 commit into from
Jun 30, 2022

Conversation

@daeyeon
Copy link
Member

@daeyeondaeyeon commented Jun 5, 2022•
edited
Loading

This removes a comment relevant to runtime checks for async_hooks.

Even if async_hooks is experimental, the check pointed by the comment
is performed as default unless --no-force-async-hooks-checks is given
from CLI arguments.

Refs: #16318
Refs: #15454 (comment)

Plus, the comment conflicts with the following L1071.

node/src/env.cc

Lines 1071 to 1076 in 45d7ca9

// Always perform async_hooks checks, not just when async_hooks is enabled.
// TODO(AndreasMadsen): Consider removing this for LTS releases.
// See discussion in https://github.com/nodejs/node/pull/15454
// When removing this, do it by reverting the commit. Otherwise the test
// and flag changes won't be included.
fields_[kCheck] = 1;

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

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 5, 2022
@daeyeondaeyeonforce-pushed the master.src.comment-220605.Sun.ec61 branch 3 times, most recently from 3d249ee to 4fd0cf8CompareJune 5, 2022 12:47
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@github-actionsgithub-actionsbot mentioned this pull request Jun 16, 2022
Open
22 tasks
@RaisinTenRaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 16, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
This was referenced Jun 17, 2022
Open
Open
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@github-actionsgithub-actionsbot mentioned this pull request Jun 19, 2022
Open
30 tasks
@daeyeon
src: remove a stale comment in async_hooks
c9a0477
This removes a comment relevant to runtime checks for `async_hooks`. Even if `async_hooks` is experimental, the check pointed by the comment is performed as default unless `--no-force-async-hooks-checks` is given from CLI arguments. Refs: nodejs#16318 Refs: nodejs#15454 (comment) Signed-off-by: Daeyeon Jeong [email protected]
@daeyeondaeyeonforce-pushed the master.src.comment-220605.Sun.ec61 branch from 4fd0cf8 to c9a0477CompareJune 19, 2022 12:19
@daeyeon
Copy link
MemberAuthor

daeyeon commented Jun 19, 2022

I've rebased this PR's codebase since the previous one includes some flaky tests such as test_buffer/test_finalizer, test-net-connect-reset-until-connected, and so on. Seemingly, the flakinesses have been reduced with recent PRs. :-)

Could you re-run the CI?

@github-actionsgithub-actionsbot mentioned this pull request Jun 20, 2022
Open
36 tasks
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

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

nodejs-github-bot commented Jun 20, 2022

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

This was referenced Jun 21, 2022
Open
Open
Open
This was referenced Jun 24, 2022
Open
Open
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 30, 2022

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

@aduh95aduh95 merged commit 3b0995e into nodejs:mainJun 30, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jun 30, 2022

Landed in 3b0995e

@aduh95aduh95 removed the needs-ci PRs that need a full CI run. label Jun 30, 2022
@github-actionsgithub-actionsbot mentioned this pull request Jul 4, 2022
Open
23 tasks
targos pushed a commit that referenced this pull request Jul 12, 2022
@daeyeon@targos
src: remove a stale comment in async_hooks
5ae30bf
This removes a comment relevant to runtime checks for `async_hooks`. Even if `async_hooks` is experimental, the check pointed by the comment is performed as default unless `--no-force-async-hooks-checks` is given from CLI arguments. Refs: #16318 Refs: #15454 (comment) Signed-off-by: Daeyeon Jeong [email protected] PR-URL: #43317 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
@targostargos mentioned this pull request Jul 12, 2022
Merged
targos pushed a commit that referenced this pull request Jul 20, 2022
@daeyeon@targos
src: remove a stale comment in async_hooks
c5a822e
This removes a comment relevant to runtime checks for `async_hooks`. Even if `async_hooks` is experimental, the check pointed by the comment is performed as default unless `--no-force-async-hooks-checks` is given from CLI arguments. Refs: #16318 Refs: #15454 (comment) Signed-off-by: Daeyeon Jeong [email protected] PR-URL: #43317 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
@daeyeon@targos
src: remove a stale comment in async_hooks
26e4dc8
This removes a comment relevant to runtime checks for `async_hooks`. Even if `async_hooks` is experimental, the check pointed by the comment is performed as default unless `--no-force-async-hooks-checks` is given from CLI arguments. Refs: #16318 Refs: #15454 (comment) Signed-off-by: Daeyeon Jeong [email protected] PR-URL: #43317 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
@targostargos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@daeyeon@guangwong
src: remove a stale comment in async_hooks
0ee391a
This removes a comment relevant to runtime checks for `async_hooks`. Even if `async_hooks` is experimental, the check pointed by the comment is performed as default unless `--no-force-async-hooks-checks` is given from CLI arguments. Refs: nodejs/node#16318 Refs: nodejs/node#15454 (comment) Signed-off-by: Daeyeon Jeong [email protected] PR-URL: nodejs/node#43317 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@lpincalpincalpinca approved these changes

@juanarboljuanarboljuanarbol approved these changes

Assignees

No one assigned

Labels

author readyPRs 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++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@daeyeon@nodejs-github-bot@aduh95@jasnell@lpinca@juanarbol@RaisinTen