Skip to content

test,worker_threads: simplify common.isMainThread #25426

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 9, 2019

Now that worker_threads do not require a flag, the logic around
loading isMainThread can be removed.

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

Now that `worker_threads` do not require a flag, the logic around
loading `isMainThread` can be removed.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 9, 2019
@Trott
Copy link
Member Author

Trott commented Jan 9, 2019

@richardlau
Copy link
Member

Is there a case for removing this from common entirely now and directly calling require('worker_threads').isMainThread from the tests?

@Trott
Copy link
Member Author

Trott commented Jan 10, 2019

Is there a case for removing this from common entirely now and directly calling require('worker_threads').isMainThread from the tests?

@richardlau Yes, probably, but that will be a larger change with more churn that will be more likely to have merge conflicts on backporting etc. So I was thinking: Do this narrower change first.

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 11, 2019
@Trott
Copy link
Member Author

Trott commented Jan 12, 2019

Landed in 9e62bb5

@Trott Trott closed this Jan 12, 2019
Trott added a commit to Trott/io.js that referenced this pull request Jan 12, 2019
Now that `worker_threads` do not require a flag, the logic around
loading `isMainThread` can be removed.

PR-URL: nodejs#25426
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 14, 2019
Now that `worker_threads` do not require a flag, the logic around
loading `isMainThread` can be removed.

PR-URL: #25426
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
Now that `worker_threads` do not require a flag, the logic around
loading `isMainThread` can be removed.

PR-URL: nodejs#25426
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
@Trott Trott deleted the my-main-thread branch January 13, 2022 22:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants