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.

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

src: remove trace_sync_io_ from env #22726

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Sep 6, 2018

This commit removes trace_sync_io_ and instead uses the options value
directly.

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

This commit removes trace_sync_io_ and instead uses the options value
directly.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 6, 2018
@danbev
Copy link
Contributor Author

danbev commented Sep 6, 2018

@danbev
Copy link
Contributor Author

danbev commented Sep 9, 2018

Rebuild of failing node-test-commit-freebsd

@danbev
Copy link
Contributor Author

danbev commented Sep 9, 2018

Landed in 82787fb.

@danbev danbev closed this Sep 9, 2018
@danbev danbev deleted the remove_trace_sync_io_ branch September 9, 2018 12:47
danbev added a commit that referenced this pull request Sep 9, 2018
This commit removes trace_sync_io_ and instead uses the options value
directly.

PR-URL: #22726
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
targos pushed a commit that referenced this pull request Sep 10, 2018
This commit removes trace_sync_io_ and instead uses the options value
directly.

PR-URL: #22726
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
ChALkeR added a commit to ChALkeR/io.js that referenced this pull request Aug 2, 2019
This reverts commit 7fa5f54.

The reverted commit breaks the logic behind --trace-sync-io, it should
be enabled only at a certain point in time, while that commit enables
it from the very start, causing warnings be printed for all sync io
instead of sync io after the first tick of the event loop as documented.

Fixes: nodejs#28913
Refs: nodejs#22726
Refs: https://nodejs.org/api/cli.html#cli_trace_sync_io
PR-URL: nodejs#28926
Trott pushed a commit to Trott/io.js that referenced this pull request Aug 4, 2019
This reverts commit 7fa5f54.

The reverted commit breaks the logic behind --trace-sync-io, it should
be enabled only at a certain point in time, while that commit enables
it from the very start, causing warnings be printed for all sync io
instead of sync io after the first tick of the event loop as documented.

Fixes: nodejs#28913
Refs: nodejs#22726
Refs: https://nodejs.org/api/cli.html#cli_trace_sync_io
PR-URL: nodejs#28926
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR pushed a commit that referenced this pull request Aug 6, 2019
This reverts commit 7fa5f54.

The reverted commit breaks the logic behind --trace-sync-io, it should
be enabled only at a certain point in time, while that commit enables
it from the very start, causing warnings be printed for all sync io
instead of sync io after the first tick of the event loop as documented.

Fixes: #28913
Refs: #22726
Refs: https://nodejs.org/api/cli.html#cli_trace_sync_io
PR-URL: #28926
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants