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.

Sign up for GitHub

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

Jump to bottom

tools: refactor license2rtf.js to ESM #43101

Merged
merged 1 commit into from
May 25, 2022
Merged

tools: refactor license2rtf.js to ESM #43101

merged 1 commit into from
May 25, 2022

Conversation

F3n67u
Copy link
Member

@F3n67u F3n67u commented May 14, 2022 •
edited
Loading

This PR tries to refactor license2rtf.js to ESM. Should still produce the same result:

$ git checkout master                                                                 
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
$ cat LICENSE | out/Release/node tools/license2rtf.js > LICENSE_ORIGINAL.rtf 
$ git checkout esm/license2rtf            
Switched to branch 'esm/license2rtf'
Your branch is up to date with 'origin/esm/license2rtf'.
$ cat LICENSE | out/Release/node tools/license2rtf.mjs > LICENSE_NEW.rtf     
$ sha256sum LICENSE_ORIGINAL.rtf LICENSE_NEW.rtf
48264d4618ee04c3347303d4de0e36d351c08ad885a9c7bf2c3b310523178325  LICENSE_ORIGINAL.rtf
48264d4618ee04c3347303d4de0e36d351c08ad885a9c7bf2c3b310523178325  LICENSE_NEW.rtf

Changes:

  • commonjs format to esm
  • use pipeline promise version instead of a serial of pipe function to improve readability

@nodejs-github-bot 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 14, 2022
@F3n67u F3n67u changed the title tools: refactor license2rtf.js to esm module tools: refactor license2rtf.js to ESM May 15, 2022
@F3n67u F3n67u force-pushed the esm/license2rtf branch 3 times, most recently from c41fa26 to e31032d Compare May 16, 2022 02:19
aduh95
aduh95 reviewed May 22, 2022
View reviewed changes
tools/license2rtf.mjs Outdated Show resolved Hide resolved
tools/license2rtf.mjs Outdated Show resolved Hide resolved
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. release Issues and PRs related to Node.js releases. and removed windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels May 22, 2022
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 25, 2022
@nodejs-github-bot 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 May 25, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43101
✔  Done loading data for nodejs/node/pull/43101
----------------------------------- PR info ------------------------------------
Title      tools: refactor license2rtf.js to ESM (#43101)
Author     Feng Yu  (@F3n67u)
Branch     F3n67u:esm/license2rtf -> nodejs:master
Labels     tools, author ready, release
Commits    1
 - tools: refactor license2rtf.js to ESM
Committers 1
 - Feng Yu 
PR-URL: https://github.com/nodejs/node/pull/43101
Reviewed-By: Antoine du Hamel 
Reviewed-By: Minwoo Jung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43101
Reviewed-By: Antoine du Hamel 
Reviewed-By: Minwoo Jung 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 14 May 2022 14:00:55 GMT
   ✔  Approvals: 2
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/43101#pullrequestreview-980957299
   ✔  - Minwoo Jung (@JungMinu): https://github.com/nodejs/node/pull/43101#pullrequestreview-984239772
   ✔  Last GitHub CI successful
   ✖  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2385162248

@nodejs-github-bot
Copy link
Collaborator

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

@RaisinTen RaisinTen added needs-ci PRs that need a full CI run. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 25, 2022
@aduh95 aduh95 removed the needs-ci PRs that need a full CI run. label May 25, 2022
@aduh95 aduh95 merged commit 30cb1bf into nodejs:master May 25, 2022
@aduh95
Copy link
Contributor

aduh95 commented May 25, 2022

Landed in 30cb1bf

@F3n67u F3n67u deleted the esm/license2rtf branch May 26, 2022 00:12
@github-actions github-actions bot mentioned this pull request May 26, 2022
Open
19 tasks
@richardlau richardlau mentioned this pull request May 26, 2022
Closed
@richardlau richardlau added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels May 26, 2022
@richardlau richardlau mentioned this pull request May 26, 2022
Merged
nodejs-github-bot pushed a commit that referenced this pull request May 26, 2022
@richardlau
Revert "tools: refactor tools/license2rtf to ESM"
331088f 
This reverts commit 30cb1bf.

PR-URL: #43214
Fixes: #43213
Refs: #43101
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@tniessen
Copy link
Member

FWIW, this was merged despite a failed Jenkins CI.

@F3n67u
Copy link
Member Author

F3n67u commented May 27, 2022

FWIW, this was merged despite a failed Jenkins CI.

Thanks for your info.

@richardlau
Copy link
Member

FWIW, this was merged despite a failed Jenkins CI.

Thanks for your info.

This was reverted in #43214 because of the failed Jenkins CI -- it was causing the CI to consistently fail on Windows.

This was referenced May 28, 2022
Open
Open
@F3n67u F3n67u mentioned this pull request May 29, 2022
Closed
This was referenced May 30, 2022
Open
Open
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
@F3n67u @danielleadams
tools: refactor tools/license2rtf to ESM
2b0d8de 
PR-URL: #43101
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
@richardlau @danielleadams
Revert "tools: refactor tools/license2rtf to ESM"
47fd3c1 
This reverts commit 30cb1bf.

PR-URL: #43214
Fixes: #43213
Refs: #43101
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 11, 2022
Merged
@RafaelGSS RafaelGSS mentioned this pull request Oct 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JungMinu JungMinu JungMinu approved these changes

@aduh95 aduh95 aduh95 approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. release Issues and PRs related to Node.js releases. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@F3n67u @nodejs-github-bot @aduh95 @tniessen @richardlau @JungMinu @RaisinTen