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.

Sign up for GitHub

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

Jump to bottom

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: [email protected] (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: 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 watilde mentioned this issue Oct 31, 2017
Closed
2 tasks
watilde added a commit to watilde/node that referenced this issue Nov 8, 2017
@watilde
tools: include remark node_modules
4d77a01 
Fixes: nodejs#16628
@refack refack mentioned this issue Nov 10, 2017
Merged
3 tasks
refack added a commit to refack/node that referenced this issue Nov 14, 2017
@refack
tools: speed up lint-md-build
61793df 
by using package-lock.json

PR-URL: nodejs#16945
Fixes: nodejs#16628
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Nov 26, 2017
@watilde @MylesBorins
tools: include remark node_modules
2e88555 
Fixes: nodejs#16628
MylesBorins pushed a commit that referenced this issue Dec 11, 2017
@refack @MylesBorins
tools: speed up lint-md-build
4d0c70a 
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@watilde watilde mentioned this issue Feb 26, 2018
Closed
2 tasks
@estrada9166 estrada9166 mentioned this issue Apr 12, 2018
Closed
2 tasks
MylesBorins pushed a commit that referenced this issue May 22, 2018
@refack @MylesBorins
tools: speed up lint-md-build
83cd22e 
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jun 14, 2018
@refack @MylesBorins
tools: speed up lint-md-build
f7b7d7e 
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
rvagg pushed a commit that referenced this issue Aug 16, 2018
@refack @rvagg
tools: speed up lint-md-build
3857e10 
by using package-lock.json

PR-URL: #16945
Fixes: #16628
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tools: speed up lint-md-build refack/node
3 participants
@watilde @maclover7 @gibfahn