nodejs
build: support `lint-js-fix` in `vcbuild.bat`
Hey @RedYetiDev, the labels you created should be called from other command in order to be executed. Feel free to use the patch I'm sending.
Another thing, this is not as important, I think lint-md-fix and lint-js-fix should go before lint-md and lint-js, just in case someone tries to run both commands at the same time.
From e45abb7e84c32d7403a196303e406ed775580261 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= <[email protected]>
Date: Mon, 20 May 2024 14:24:40 +0300
Subject: [PATCH 1/1] build: fix lint-md-fix and lint-js-fix
---
vcbuild.bat | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/vcbuild.bat b/vcbuild.bat
index f07711fab7..1b71db935d 100644
--- a/vcbuild.bat
+++ b/vcbuild.bat
@@ -707,11 +707,11 @@ goto lint-js
goto lint-js
:lint-js
-if not defined lint_js goto lint-md-build
+if not defined lint_js goto lint-js-fix
if not exist tools\node_modules\eslint goto no-lint
echo running lint-js
%node_exe% tools\node_modules\eslint\bin\eslint.js --cache --max-warnings=0 --report-unused-disable-directives --rule "linebreak-style: 0" .eslintrc.js benchmark doc lib test tools
-goto lint-md-build
+goto lint-js-fix
:lint-js-fix
if not defined lint_js_fix goto lint-md-build
@@ -731,7 +731,7 @@ echo "Deprecated no-op target 'lint_md_build'"
goto lint-md
:lint-md
-if not defined lint_md goto format-md
+if not defined lint_md goto lint-md-fix
echo Running Markdown linter on docs...
SETLOCAL ENABLEDELAYEDEXPANSION
set lint_md_files=
@@ -742,7 +742,7 @@ for /D %%D IN (doc\*) do (
)
%node_exe% tools\lint-md\lint-md.mjs %lint_md_files%
ENDLOCAL
-goto format-md
+goto lint-md-fix
:lint-md-fix
if not defined lint_md_fix goto format-md
--
2.43.0.windows.1
Another thing, this is not as important, I think
lint-md-fixandlint-js-fixshould go beforelint-mdandlint-js, just in case someone tries to run both commands at the same time.
That won't happen, as there can only be one %1 argument (IIUC)
That won't happen, as there can only be one
%1argument (IIUC)
Theoretically, if you call vcbuild.bat lint-js lint-js-fix it will do both. The value of %1 is changed with the usage of shift command here. As I said, this part is a suggestion.
The patch file I provided enables the 2 new commands. Running vcbuild.bat lint-js-fix does nothing for me locally unless the patch is applied. I do not see running lint-js-fix which should be there.
That won't happen, as there can only be one
%1argument (IIUC)Theoretically, if you call
vcbuild.bat lint-js lint-js-fixit will do both. The value of%1is changed with the usage ofshiftcommand here. As I said, this part is a suggestion.The patch file I provided enables the 2 new commands. Running
vcbuild.bat lint-js-fixdoes nothing for me locally unless the patch is applied. I do not seerunning lint-js-fixwhich should be there.
Good to know, thanks (I'm not a batch file expert)
I've made your suggestions. Sorry I brushed your comment off initially, I'm not a batch file expert and I didn't quite understand how goto worked.
To summarize, I darn goofed and --fix isn't supported by the markdown linter.
Could someone with a Windows machine validate that this works so this can land?
The script is being called correctly, so this PR is good.
But I'm getting an error (which is not related to this PR) that is:
$ node.exe tools\node_modules\eslint\bin\eslint.js --cache --max-warnings=0 --report-unused-disable-directives --rule "linebreak-style: 0" .eslintrc.js benchmark doc lib test tools
Oops! Something went wrong! :(
ESLint: 8.57.0
C:\Users\vinic\Desktop\Projects\node\tools\node_modules\eslint\node_modules\eslint:1
..
^
SyntaxError: Unexpected token '.'
at internalCompileFunction (node:internal/vm:128:18)
at wrapSafe (node:internal/modules/cjs/loader:1279:20)
at Module._compile (node:internal/modules/cjs/loader:1331:27)
at Module._extensions..js (node:internal/modules/cjs/loader:1426:10)
at Module.load (node:internal/modules/cjs/loader:1205:32)
at Module._load (node:internal/modules/cjs/loader:1021:12)
at Module.require (node:internal/modules/cjs/loader:1230:19)
at require (node:internal/modules/helpers:179:18)
at Object.<anonymous> (C:\Users\vinic\Desktop\Projects\node\tools\node_modules\eslint\node_modules\eslint-plugin-jsdoc\dist\rules\checkExamples.cjs:8:39)
at Module._compile (node:internal/modules/cjs/loader:1368:14)
Any recommendations on what to do here?
@H4ad eslint is a symlink that doesn't work by default in Git. To fix this issue, the repo should be cloned as follows
git clone -c core.symlinks=true <repository_url>
Commit Queue failed
- Loading data for nodejs/node/pull/53046 ✔ Done loading data for nodejs/node/pull/53046 ----------------------------------- PR info ------------------------------------ Title build: support `lint-js-fix` in `vcbuild.bat` (#53046) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch RedYetiDev:patch-32 -> nodejs:main Labels windows, build, author ready, needs-ci Commits 4 - build: support `lint-md-fix` and `lint-js-fix` in `vcbuild.bat` - Update vcbuild.bat - Update vcbuild.bat - Update vcbuild.bat Committers 1 - GitHubhttps://github.com/nodejs/node/actions/runs/9208503129PR-URL: https://github.com/nodejs/node/pull/53046 Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow Reviewed-By: Vinícius Lourenço Claro Cardoso ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/53046 Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow Reviewed-By: Vinícius Lourenço Claro Cardoso -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 18 May 2024 13:18:20 GMT ✔ Approvals: 3 ✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53046#pullrequestreview-2064964306 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53046#pullrequestreview-2065008304 ✔ - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/53046#pullrequestreview-2073766737 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-05-19T07:10:15Z: https://ci.nodejs.org/job/node-test-pull-request/59298/ ⚠ Commits were pushed after the last Full PR CI run: ⚠ - Update vcbuild.bat ⚠ - Update vcbuild.bat - Querying data for job/node-test-pull-request/59298/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
@RedYetiDev can you please rebase to get rid of the merge commit that the CI is choking on?