tools: yaml lint command in vcbuild script
Hello everyone, I'm opening this PR hoping it could be an enhancement for Windows contributors.
To give this a little context, when I was working on #48184 , I noticed that lint-yaml was run by a Github Action, but it was not present in the vcbuild.bat script that I used for building and linting files.
Following things were changed:
-
lint-yamlwas added as a command in vcbuild script and it is executed through the Makefile corresponding command. -
lint-yamlstep was also added and enabled for execution ontestandlintcommands. - Made
wsl makepreferred for executing Makefile command onlint-cppandlint-yamlbecause of Square brackets not being recognizable by Windows as a command. - Added
lint-yamlreference to thehelpcommand. - Deleted an
eslinttext file in order to makelint-jscommand work again.lint-jsexecution would look at that file, throw a Syntax Error thatonFatalErrorineslint.jswould catch and make the linter execution stop. - Disabled
new-linestype checking from.yamllint.yamlin order to make linter work on Windows even though the execution was started on a Unix system (such as WSL). I think this could be re-enabled if we are able to use GNU Make instead of WSL to lint those files.
I also have two questions:
- I noticed that
lintcommand would skip the build process if it's already built, is this a behaviour that could be replicated for eachlint-Xcommand? - Regarding change no. 3, what's the best path to follow here? Makefile command accounting for Windows systems that can call it or just stick with
WSL makeand get rid of the GNU Make option?
Thanks for your attention!
@nodejs/build
- Deleted an
eslinttext file in order to makelint-jscommand work again.lint-jsexecution would look at that file, throw a Syntax Error thatonFatalErrorineslint.jswould catch and make the linter execution stop.
I might be misunderstanding what's going on here, but I think that file will likely be re-added the next time eslint is updated. Regardless, lint-js should not be linting files that do not have a .*js suffix, so I'm not sure why it was causing a problem. (Like I said, I might just be not understanding what's going on here.)
- Deleted an
eslinttext file in order to makelint-jscommand work again.lint-jsexecution would look at that file, throw a Syntax Error thatonFatalErrorineslint.jswould catch and make the linter execution stop.I might be misunderstanding what's going on here, but I think that file will likely be re-added the next time
eslintis updated. Regardless,lint-jsshould not be linting files that do not have a .*js suffix, so I'm not sure why it was causing a problem. (Like I said, I might just be not understanding what's going on here.)
Thank you for your response :)
I've investigated this problem more in depth and I found out that eslint text file wasn't the real problem. I'm sorry that I made the wrong assumptions.
I'll try to reconstruct my steps:
- launch command
lint-jsfromvcbuild.batfile -> ImmediateSyntax Erroron thateslinttext file. In the error stack though there's a reference totools\node_modules\eslint\node_modules\eslint-plugin-jsdoc\dist\rules\checkExamples.js:8:15which is arequire("eslint") - when that lines executes it always points back to the
eslinttext file, instead of actually pointing totools\node_modules\eslint\lib\eslint\eslint.js - If I manually make the require go look for
tools\node_modules\eslint\lib\eslint\eslint.js(with relative paths) then linting Javascript files works fine.
I wanted to bring this up before making any changes because I have no evidence this could work on other OSs where I suppose everything works great already.
Is require working differently between OSs? If so, do you think that replacing that require with relative paths could be a good solution?
Thank you again for your time and consideration!
This could use some more reviews. @nodejs/linting @nodejs/platform-windows
- Deleted an
eslinttext file in order to makelint-jscommand work again.lint-jsexecution would look at that file, throw a Syntax Error thatonFatalErrorineslint.jswould catch and make the linter execution stop.I might be misunderstanding what's going on here, but I think that file will likely be re-added the next time
eslintis updated. Regardless,lint-jsshould not be linting files that do not have a .*js suffix, so I'm not sure why it was causing a problem. (Like I said, I might just be not understanding what's going on here.)
This file is a symbolic link. On Windows, symlinks are disabled by default and must be enabled manually.
Sorry for the late reply.
I've reverted the changes on the Eslint file. Enabling Symlinks by activating Developer Mode on Windows actually made lint-js command work.
Thanks for the suggestion!
This needs a rebase.
This needs a rebase.
Done, thank you for the reminder! I've been away from my Windows setup for a bit.