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 targos and LiviaMedeiros 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

F3n67u commented May 29, 2022 •

edited

Loading

F3n67u commented May 29, 2022

nodejs-github-bot commented May 29, 2022

nodejs-github-bot commented Jun 30, 2022

nodejs-github-bot commented Jul 2, 2022

F3n67u commented Jul 3, 2022 •

edited

Loading

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

F3n67u commented Jul 4, 2022

LiviaMedeiros commented Jul 7, 2022

tools: refactor tools/license2rtf to ESM #43232

tools: refactor tools/license2rtf to ESM #43232

Conversation

F3n67u commented May 29, 2022 • edited Loading

Windows

Before the fix

After fix

MacOS

Before the fix

After fix

F3n67u commented May 29, 2022

nodejs-github-bot commented May 29, 2022

nodejs-github-bot commented Jun 30, 2022

nodejs-github-bot commented Jul 2, 2022

F3n67u commented Jul 3, 2022 • edited Loading

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

nodejs-github-bot commented Jul 3, 2022

F3n67u commented Jul 4, 2022

LiviaMedeiros commented Jul 7, 2022

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