build,deps: replace cjs-module-lexer with merve#61456
build,deps: replace cjs-module-lexer with merve#61456nodejs-github-bot merged 2 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
ae6ab98 to
5e69495
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61456 +/- ##
==========================================
- Coverage 89.85% 89.78% -0.07%
==========================================
Files 671 673 +2
Lines 203166 203822 +656
Branches 39057 39183 +126
==========================================
+ Hits 182554 183001 +447
- Misses 12967 13139 +172
- Partials 7645 7682 +37
🚀 New features to boost your workflow:
|
9bfb358 to
1da16dd
Compare
66e99b4 to
d687c0a
Compare
|
Where does this new dep live? |
d687c0a to
b2ec704
Compare
|
Per current process it would make sense to move merve into the Node.js org as well. I can then add a note to the cjs-module-lexer readme that future bugs should be posted there. I'd value also having permissions to the new repo to continue to provide support. |
lemire
left a comment
There was a problem hiding this comment.
Next step, name a library after your cat.
There was a problem hiding this comment.
Please also update these references to the removed cjs-module-lexer:
Line 190 in f700203
- and
Line 630 in f700203
andLine 1285 in f700203
Line 1323 in f700203
- doc/contributing/maintaining/maintaining-cjs-module-lexer.md
- References to the above maintaining doc
node/doc/contributing/maintaining/maintaining-dependencies.md
Lines 241 to 246 in f700203
- and
node/.github/workflows/tools.yml
Line 22 in f700203
node/.github/workflows/tools.yml
Lines 108 to 115 in f700203
richardlau
left a comment
There was a problem hiding this comment.
Same as #61456 (comment), I would prefer the dependency was in the Node.js org (as the dep it is replacing, cjs-module-lexer, is).
|
cc @nodejs/tsc are we all ok with moving https://github.com/anonrig/merve to node.js organization? my only ask is to keep the name and don't change it in the future. |
Yeah, just follow the process described in https://github.com/nodejs/admin/blob/main/transfer-repo-into-the-org.md. Happy to help if needed! |
Appreciate any help @aduh95, I'll give you admin rights to the repo. |
|
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
|
let's land this pull request and i'll follow up with the changes to move the repo to nodejs organization and update references. |
|
Landed in 37e4004 |
PR-URL: #61456 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Richard Lau <richard.lau@ibm.com>
Initially started a couple of years ago, the new cjs module lexer is 25% faster for cold importing cjs files. This change introduces a new dependency and removes the old one.
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1784/console