tools: refactor `tools/license2rtf` to ESM #43232

F3n67u · 2022-05-29T04:51:00Z

This pr reimplement #43101: refactor license2rtf.js to ESM.

The last pr(43101) is reverted by #43214 because it causes windows ci failure.

Windows

Before the fix

C:\Users\Administrator\dev\node>git log -1           
commit c9f5078d8cb601624caeec656823445475050bc1 (HEAD -> master)
Author: Feng <[email protected]>
Date:   Sun May 29 12:14:26 2022 +0800

    Revert "Revert "tools: refactor `tools/license2rtf` to ESM""
    
    This reverts commit 331088f4a450e29f3ea8a28a9f98ccc9f8951386.

C:\Users\Administrator\dev\node>git --no-pager diff                                             

C:\Users\Administrator\dev\node>.\Release\node.exe tools\license2rtf.mjs < LICENSE > license.rtf

C:\Users\Administrator\dev\node>echo Exit Code is %errorlevel%
Exit Code is 13

The exit code of .\Release\node.exe tools\license2rtf.mjs < LICENSE > license.rtf command is 13 which cause windows ci failure: Failed to generate license.rtf(#43213).

The errorlevel variable check of windows build is at:

node/vcbuild.bat

Line 428 in ca0044b

if errorlevel 1 echo Failed to generate license.rtf&goto exit

After fix

C:\Users\Administrator\dev\node>git --no-pager diff
diff --git a/tools/license2rtf.mjs b/tools/license2rtf.mjs
index 0772b161ed..6f07e57c7f 100644
--- a/tools/license2rtf.mjs
+++ b/tools/license2rtf.mjs
@@ -289,11 +289,16 @@ class RtfGenerator extends Stream {
 stdin.setEncoding('utf-8');
 stdin.resume();
 
-await pipeline(
-  stdin,
-  new LineSplitter(),
-  new ParagraphParser(),
-  new Unwrapper(),
-  new RtfGenerator(),
-  stdout,
-);
+async function main() {
+  await pipeline(
+    stdin,
+    new LineSplitter(),
+    new ParagraphParser(),
+    new Unwrapper(),
+    new RtfGenerator(),
+    stdout,
+  );
+}
+
+main().catch(console.error);
+

C:\Users\Administrator\dev\node>.\Release\node.exe tools\license2rtf.mjs < LICENSE > license.rtf

C:\Users\Administrator\dev\node>echo Exit Code is %errorlevel%
Exit Code is 0

The exit code of .\Release\node.exe tools\license2rtf.mjs < LICENSE > license.rtf command is 0.

MacOS

Before the fix

$ git --no-pager log -1          
commit 255c643e136a1aa11c4cbb6667f76624e85b4721 (HEAD -> master, revert-revert-esm-license2rtf)
Author: Feng Yu <[email protected]>
Date:   Sun May 29 12:19:04 2022 +0800

    Revert "Revert "tools: refactor `tools/license2rtf` to ESM""
    
    This reverts commit 331088f4a450e29f3ea8a28a9f98ccc9f8951386.
$ ./node tools/license2rtf.mjs < LICENSE > license.rtf 
$ echo $?
13

The exit code of ./node tools/license2rtf.mjs < LICENSE > license.rtf command is 13.

After fix

$ git --no-pager diff            
diff --git a/tools/license2rtf.mjs b/tools/license2rtf.mjs
index 0772b161ed..4cdca33b60 100644
--- a/tools/license2rtf.mjs
+++ b/tools/license2rtf.mjs
@@ -289,11 +289,15 @@ class RtfGenerator extends Stream {
 stdin.setEncoding('utf-8');
 stdin.resume();
 
-await pipeline(
-  stdin,
-  new LineSplitter(),
-  new ParagraphParser(),
-  new Unwrapper(),
-  new RtfGenerator(),
-  stdout,
-);
+async function main() {
+  await pipeline(
+    stdin,
+    new LineSplitter(),
+    new ParagraphParser(),
+    new Unwrapper(),
+    new RtfGenerator(),
+    stdout,
+  );
+}
+
+main().catch(console.error);
$ ./node tools/license2rtf.mjs < LICENSE > license.rtf 
$ echo $?                                              
0

The exit code of ./node tools/license2rtf.mjs < LICENSE > license.rtf command is 0.

F3n67u · 2022-05-29T05:05:35Z

I leave the revert-revert commit untouched intentionally to make code review easier. so the commit message ci check will fail.

once this pr has enough approval, I will squash those two commits to one commit.

tools/license2rtf.mjs

nodejs-github-bot · 2022-05-29T14:25:15Z

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

tools/license2rtf.mjs

nodejs-github-bot · 2022-06-30T16:22:07Z

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

nodejs-github-bot · 2022-07-02T12:11:57Z

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

F3n67u · 2022-07-03T02:06:29Z

Please help to trigger another ci run.

The last ci failure is caused by flaky test-stream-finished, this test has already been fixed in main branch.

nodejs-github-bot · 2022-07-03T06:44:35Z

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

nodejs-github-bot · 2022-07-03T08:01:57Z

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

nodejs-github-bot · 2022-07-03T11:28:31Z

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

nodejs-github-bot · 2022-07-03T12:53:14Z

Commit Queue failed

- Loading data for nodejs/node/pull/43232
✔  Done loading data for nodejs/node/pull/43232
----------------------------------- PR info ------------------------------------
Title      tools: refactor `tools/license2rtf` to ESM (#43232)
Author     Feng Yu  (@F3n67u)
Branch     F3n67u:revert-revert-esm-license2rtf -> nodejs:main
Labels     windows, build, tools, author ready, needs-ci
Commits    1
 - tools: refactor `tools/license2rtf` to ESM
Committers 1
 - Feng Yu 
PR-URL: https://github.com/nodejs/node/pull/43232
Refs: https://github.com/nodejs/node/issues/43213
Reviewed-By: Darshan Sen 
Reviewed-By: LiviaMedeiros 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43232
Refs: https://github.com/nodejs/node/issues/43213
Reviewed-By: Darshan Sen 
Reviewed-By: LiviaMedeiros 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - tools: refactor `tools/license2rtf` to ESM
   ℹ  This PR was created on Sun, 29 May 2022 04:51:00 GMT
   ✔  Approvals: 2
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/43232#pullrequestreview-988629243
   ✔  - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/43232#pullrequestreview-995051304
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-07-03T11:28:31Z: https://ci.nodejs.org/job/node-test-pull-request/45068/
- Querying data for job/node-test-pull-request/45068/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2605126551

F3n67u · 2022-07-04T15:03:10Z

Please help to land this pr, thanks in advance

This reverts commit 331088f. PR-URL: #43232 Refs: #43213 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

LiviaMedeiros · 2022-07-07T18:39:06Z

Landed in a420995

This reverts commit 331088f. PR-URL: #43232 Refs: #43213 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

This reverts commit 331088f. PR-URL: nodejs/node#43232 Refs: nodejs/node#43213 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels May 29, 2022

F3n67u force-pushed the revert-revert-esm-license2rtf branch from 894af9c to 7eff826 Compare May 29, 2022 04:55

F3n67u changed the title ~~Revert revert esm license2rtf~~ tools: refactor tools/license2rtf to ESM May 29, 2022

F3n67u marked this pull request as ready for review May 29, 2022 05:02

F3n67u mentioned this pull request May 29, 2022

Windows build break: Failed to generate license.rtf #43213

Closed

F3n67u force-pushed the revert-revert-esm-license2rtf branch 4 times, most recently from 3841caa to 9d9b1f8 Compare May 29, 2022 05:58

RaisinTen approved these changes May 29, 2022

View reviewed changes

tools/license2rtf.mjs Outdated Show resolved Hide resolved

F3n67u force-pushed the revert-revert-esm-license2rtf branch from 9d9b1f8 to 1376a8b Compare May 29, 2022 14:23

F3n67u requested a review from RaisinTen May 29, 2022 14:24

RaisinTen approved these changes May 29, 2022

View reviewed changes

RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 29, 2022

targos reviewed May 29, 2022

View reviewed changes

tools/license2rtf.mjs Outdated Show resolved Hide resolved

RaisinTen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 29, 2022

This was referenced May 30, 2022

CI Reliability 2022-05-30 nodejs/reliability#293

Open

CI Reliability 2022-05-31 nodejs/reliability#294

Open

CI Reliability 2022-06-01 nodejs/reliability#295

Open

F3n67u force-pushed the revert-revert-esm-license2rtf branch 2 times, most recently from 20e5956 to cb0ee5d Compare June 1, 2022 01:22

F3n67u requested review from LiviaMedeiros and targos June 1, 2022 01:24

This was referenced Jun 2, 2022

CI Reliability 2022-06-02 nodejs/reliability#296

Open

CI Reliability 2022-06-03 nodejs/reliability#297

Open

F3n67u force-pushed the revert-revert-esm-license2rtf branch from 7749cc7 to 34b905d Compare June 30, 2022 16:16

F3n67u added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2022

F3n67u added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 3, 2022

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 3, 2022

github-actions bot mentioned this pull request Jul 4, 2022

CI Reliability 2022-07-04 nodejs/reliability#317

Open

23 tasks

targos approved these changes Jul 5, 2022

View reviewed changes

targos 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. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 5, 2022

LiviaMedeiros closed this Jul 7, 2022

github-actions bot mentioned this pull request Jul 8, 2022

CI Reliability 2022-07-08 nodejs/reliability#318

Open

16 tasks

F3n67u deleted the revert-revert-esm-license2rtf branch July 8, 2022 00:21

targos mentioned this pull request Jul 12, 2022

v18.6.0 release proposal #43789

Merged

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Uh oh!

tools: refactor tools/license2rtf to ESM #43232

tools: refactor tools/license2rtf to ESM #43232

Uh oh!

Conversation

F3n67u commented May 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Windows

Before the fix

After fix

MacOS

Before the fix

After fix

Uh oh!

F3n67u commented May 29, 2022

Uh oh!

Uh oh!

nodejs-github-bot commented May 29, 2022

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 30, 2022

Uh oh!

nodejs-github-bot commented Jul 2, 2022

Uh oh!

F3n67u commented Jul 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 3, 2022

Uh oh!

nodejs-github-bot commented Jul 3, 2022

Uh oh!

nodejs-github-bot commented Jul 3, 2022

Uh oh!

nodejs-github-bot commented Jul 3, 2022

Uh oh!

F3n67u commented Jul 4, 2022

Uh oh!

LiviaMedeiros commented Jul 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tools: refactor `tools/license2rtf` to ESM #43232

tools: refactor `tools/license2rtf` to ESM #43232

F3n67u commented May 29, 2022 •

edited

Loading

F3n67u commented Jul 3, 2022 •

edited

Loading