Skip to content
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

node-test-linter npm installs remark-cli for each build #16628

Closed
gibfahn opened this issue Oct 30, 2017 · 2 comments · Fixed by #16945
Closed

node-test-linter npm installs remark-cli for each build #16628

gibfahn opened this issue Oct 30, 2017 · 2 comments · Fixed by #16945
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.

Comments

@gibfahn
Copy link
Member

gibfahn commented Oct 30, 2017

  • Version:
  • Platform: CI
  • Subsystem: build

Look at https://ci.nodejs.org/job/node-test-linter/lastSuccessfulBuild/console

if [ ! -d tools/remark-cli/node_modules ]; then \
	cd tools/remark-cli && ../.././node ../.././deps/npm/bin/npm-cli.js install; fi
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.1.2 (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.1.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"freebsd","arch":"x64"})

added 272 packages in 21.24s
if [ ! -d tools/remark-preset-lint-node/node_modules ]; then \
	cd tools/remark-preset-lint-node && ../.././node ../.././deps/npm/bin/npm-cli.js install; fi
npm notice created a lockfile as package-lock.json. You should commit this file.
added 48 packages in 4.735s
Running Markdown linter...

It's only taking 25s each time, but it seems wasteful. I know this is how Travis etc. work, so maybe this should just be closed, but I miss sub two minute lint jobs.

@gibfahn gibfahn added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. labels Oct 30, 2017
@maclover7
Copy link
Contributor

Hmm, looks like node_modules is git checked-in for ESLint, but not for Remark... should Remark also do this?

cc @watilde

@watilde
Copy link
Contributor

watilde commented Oct 31, 2017

Initially, I did include node_modules at #12756 but the diff was too huge to review so I excluded them as requested. I will make a patch to add them again.

watilde added a commit to watilde/node that referenced this issue Nov 8, 2017
refack added a commit to refack/node that referenced this issue Nov 14, 2017
by using package-lock.json

PR-URL: nodejs#16945
Fixes: nodejs#16628
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Nov 26, 2017
MylesBorins pushed a commit that referenced this issue Dec 11, 2017
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue May 22, 2018
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 14, 2018
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this issue Aug 16, 2018
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants