Skip to content

process: refactor execution - #40664

Merged
aduh95 merged 1 commit into
nodejs:masterfrom
VoltrexKeyva:refactor-process-execution
Nov 6, 2021
Merged

process: refactor execution#40664
aduh95 merged 1 commit into
nodejs:masterfrom
VoltrexKeyva:refactor-process-execution

Conversation

@VoltrexKeyva

@VoltrexKeyvaVoltrexKeyva commented Oct 30, 2021

Copy link
Copy Markdown
Contributor

• Removed unreachable code of the evalModule() function as an early error is thrown when the print parameter is a truthy value.
• Make use of the nullish coalescing operator.

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Oct 30, 2021
@VoltrexKeyvaVoltrexKeyva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 30, 2021
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 30, 2021
@nodejs-github-bot

This comment has been minimized.

Comment threadlib/internal/process/execution.js Outdated
Comment threadlib/internal/process/execution.js Outdated
@VoltrexKeyva
VoltrexKeyvaforce-pushed the refactor-process-execution branch 2 times, most recently from 38f859f to ccbb9a7CompareOctober 30, 2021 12:44
@aduh95

Copy link
Copy Markdown
Contributor

For reference, throwing when trying to print the result of an ES module was added in #30370 (comment).

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

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

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

• Removed unreachable code of the `evalModule()` function as an early
error is thrown when the `print` parameter is a truthy value.
• Make use of the nullish coalescing operator.
@VoltrexKeyva
VoltrexKeyvaforce-pushed the refactor-process-execution branch from ccbb9a7 to ab93dabCompareNovember 3, 2021 13:24
@nodejs-github-bot

This comment has been minimized.

@VoltrexKeyvaVoltrexKeyva removed the needs-ci PRs that need a full CI run. label Nov 3, 2021
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@VoltrexKeyvaVoltrexKeyva added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 6, 2021
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 6, 2021
@github-actions

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/40664
✔ Done loading data for nodejs/node/pull/40664
----------------------------------- PR info ------------------------------------
Title process: refactor execution (#40664)
Author Voltrex (@VoltrexMaster)
Branch VoltrexMaster:refactor-process-execution -> nodejs:master
Labels process, author ready
Commits 1
- process: refactor execution
Committers 1
- voltrexmaster PR-URL: https://github.com/nodejs/node/pull/40664
Reviewed-By: Antoine du Hamel Reviewed-By: Luigi Pinca Reviewed-By: Minwoo Jung ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40664
Reviewed-By: Antoine du Hamel Reviewed-By: Luigi Pinca Reviewed-By: Minwoo Jung --------------------------------------------------------------------------------
⚠ Commits were pushed since the last review:
⚠ - process: refactor execution
ℹ This PR was created on Sat, 30 Oct 2021 09:46:57 GMT
✔ Approvals: 3
✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/40664#pullrequestreview-793574274
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/40664#pullrequestreview-793760306
✔ - Minwoo Jung (@JungMinu): https://github.com/nodejs/node/pull/40664#pullrequestreview-794778135
✔ Last GitHub Actions successful
ℹ Last Full PR CI on 2021-11-04T19:22:57Z: https://ci.nodejs.org/job/node-test-pull-request/40709/
- Querying data for job/node-test-pull-request/40709/
✔ Build data downloaded
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1429175442

@github-actionsgithub-actionsBot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 6, 2021
@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 6, 2021
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 6, 2021
@aduh95
aduh95 merged commit 549a4c9 into nodejs:masterNov 6, 2021
@aduh95

Copy link
Copy Markdown
Contributor

Landed in 549a4c9

@VoltrexKeyva
VoltrexKeyva deleted the refactor-process-execution branch November 6, 2021 15:16
@aduh95aduh95 mentioned this pull request Nov 6, 2021
targos pushed a commit that referenced this pull request Nov 8, 2021
• Removed unreachable code of the `evalModule()` function as an early
error is thrown when the `print` parameter is a truthy value.
• Make use of the nullish coalescing operator.
PR-URL: #40664
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@targostargos mentioned this pull request Nov 8, 2021
BethGriggs pushed a commit that referenced this pull request Nov 25, 2021
• Removed unreachable code of the `evalModule()` function as an early
error is thrown when the `print` parameter is a truthy value.
• Make use of the nullish coalescing operator.
PR-URL: #40664
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Nov 26, 2021
1 task
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.processIssues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@VoltrexKeyva@nodejs-github-bot@aduh95@lpinca@JungMinu@Mesteery