build: stop pre-compiling lint-md

[RedYetiDev]

IIUC there wasn't any discussion in #40180 and related PRs about why this file is precompiled locally. Similar to eslint (#54231), it always has been...

This PR stops the pre-compilation of this file, similar to #54231 with eslint.

Similarly, @dependabot will manage updates.

[nodejs-github-bot]

Review requested:

• @nodejs/actions
• @nodejs/security-wg

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.42%. Comparing base (53b1050) to head (e63b9af).
Report is 43 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #55266   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files         654      654           
  Lines      187552   187552           
  Branches    36087    36087           
=======================================
+ Hits       165839   165843    +4     
- Misses      14950    14952    +2     
+ Partials     6763     6757    -6     

see 33 files with indirect coverage changes

[ovflowd]

Let me see if I understood this correctly. You are adding dependabot flow for automatic updates of the lint-md dependencies and removing it from the regular GitHub actions flow?

And you also simplified the oh my fucking god (https://github.com/RedYetiDev/node/blob/main/tools/lint-md/lint-md.mjs) this is huge O.o

[ovflowd]

Nice cleanup of tech debt -- as far as I'm concerned, LGTM!

[nodejs-github-bot]

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

[RedYetiDev]

Let me see if I understood this correctly. You are adding dependabot flow for automatic updates of the lint-md dependencies and removing it from the regular GitHub actions flow?

Exactly!

[ovflowd]

And you also simplified the oh my fucking god (RedYetiDev/node@main/tools/lint-md/lint-md.mjs) this is huge O.o

I know this is just due to the fact we were using Rollup, but still, something definitely unnecessary nowadays. And it is good to remove rollup.

[targos]

Note that this probably breaks make test when run without an internet connection. But I guess that ship has already sailed when eslint/node_modules was removed.

[RedYetiDev]

Note that this probably breaks make test when run without an internet connection.

I have nothing to back this up, but I'd assume that other tests may also break without internet, regardless of this?

[RedYetiDev]

And you also simplified the oh my fucking god (RedYetiDev/node@main/tools/lint-md/lint-md.mjs) this is huge O.o

It's not really a simplification. The .src.mjs file was moved to that file.

[targos]

Note that this probably breaks make test when run without an internet connection.

I have nothing to back this up, but I'd assume that other tests may also break without internet, regardless of this?

Maybe, but not so long ago, we were careful not to break this use case (that's why unit tests that require internet are put in the separate test/internet folder).

[targos]

That's also why we have lint-py-build and lint-yaml-build, which are not run automatically.

[RedYetiDev]

Okay, so, what do you recommend? IMHO this is okay as is, and if that changes, it can always be reverted?

[targos]

I'd like to ping the people who care about this concern, but I don't remember who they are.

[richardlau]

Note that this probably breaks make test when run without an internet connection. But I guess that ship has already sailed when eslint/node_modules was removed.

I'd like to ping the people who care about this concern, but I don't remember who they are.

I think that would either be @nodejs/distros (I know for example that RHEL's build system (for packages) has no internet access) and/or perhaps people in countries with potentially restricted Internet access (e.g. China).

[kapouer]

Distributions don't need those linters to build Node properly, so it's fine to not include them in the release tarball.

[RafaelGSS]

I'd like to ping the people who care about this concern, but I don't remember who they are.

IIRC @aduh95

[RedYetiDev]

Also FWIW lint-md already required internet after eslint was removed locally, as lint-md called lint-js-md.

[RedYetiDev]

There haven't been any objections by anyone here or that has been pinged, can this land?

[nodejs-github-bot]

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

[ovflowd]

I think another approval is required.

[RedYetiDev]

I think another approval is required.

One approval is enough as it's been over a week. However, the CI needs to pass first

[nodejs-github-bot]

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

[aduh95]

I think another approval is required.

One approval is enough as it's been over a week. However, the CI needs to pass first

That's correct, worth noting that @trivikr has also approved (there have been pushes since that approval, but until it's dismissed, it still counts)

[RedYetiDev]

These CI deadlocks are terrible 😅. I'm planning to some investigation into them over the weekend. For now, can someone resume the osx-arm build? Thanks!

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 5ce3d10