-
Notifications
You must be signed in to change notification settings - Fork 30k
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.
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
[WIP]tools: remove closure_linter to eslint on windows #1685
Conversation
R=@iojs/build? |
when does closure_linter get removed from the tree? is this the last use of it? also, @yosuke-furukawa has jenkins creds now, as do most other collaborators, it's probably just the newest collaborators that aren't set up now, they're welcome to ping me directly if they're not getting creds quick enough |
I don't think Windows does "%config%\iojs" tools\eslint\bin\eslint.js src lib --reset --quiet Also, I couldn't get |
Oh, and let's not forget about the internal dir: "%config%\iojs" tools\eslint\bin\eslint.js src lib lib\internal --reset --quiet |
Nevermind, it seems specifying a directory makes eslint recursive automatically. so |
I don't think you have to quote the command. LGTM after reverting to just specifying directories -- tested on one of the buildslaves (2012). |
@jbergstroem did you test |
@silverwind I replaced the old lines in vcbuild.bat with above and ran |
Alright, LGTM then. |
jslint does not print error like I will land this. |
PR-URL: #1685 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Roman Reiss <me@silverwind.io>
Landed. 966acb9 Thank you for review. |
Minor nit I didn't notice earlier (sorry): path separator should be |
Is there any practical reason to use backslashes? I mean if io.js/windows supports forward slashes, it might make sense to use them everywhere for consistency reasons. |
PR-URL: nodejs#1685 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Roman Reiss <me@silverwind.io>
This is a work in progress.
I found an error on Windows CI. gjslint still exists on vbuild.bat.
BUT, I don't have a Windows environment... So I have not confirmed this yet.
If I got our Jenkins permission, I will run the tests on Windows.