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

tools: speed up lint-md-build #16945

Merged
merged 0 commits into from
Nov 14, 2017
Merged

tools: speed up lint-md-build #16945

merged 0 commits into from
Nov 14, 2017

Conversation

refack
Copy link
Contributor

@refack refack commented Nov 10, 2017

with a hydrated cache running npm install while a package-lock.json is present yields an 80% speedup.

d:\code\node\tools\remark-cli$ git clean -xfd
Removing node_modules/
Removing package-lock.json

d:\code\node\tools\remark-cli$ npm i --production
added 272 packages in 16.497s

d:\code\node\tools\remark-cli$ rmdir /q /s node_modules\

d:\code\node\tools\remark-cli$ npm i --production
added 157 packages in 3.538s

Fixes: #16628

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Nov 10, 2017
@refack
Copy link
Contributor Author

refack commented Nov 10, 2017

/CC @wooorm @nodejs/build

@refack
Copy link
Contributor Author

refack commented Nov 10, 2017

@gibfahn
Copy link
Member

gibfahn commented Nov 10, 2017

Seems like a reasonable solution to me.

Copy link
Member

@watilde watilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@refack
Copy link
Contributor Author

refack commented Nov 11, 2017

I'm just following @zkat's posts, they're the magician behind this. (Can't wait for cipm)

Copy link
Contributor

@XadillaX XadillaX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Trott
Copy link
Member

Trott commented Nov 13, 2017

You probably already did this, but just in case: If we have a documented update process for these things, make sure it removes the package-lock.json and replaces it with a new one. I see, for example, that remark-cli is now up to 4.0.0 and we might want to update soon, I imagine.

@refack refack closed this Nov 14, 2017
@refack refack merged commit 61793df into nodejs:master Nov 14, 2017
@refack refack deleted the mdlint-on-tarball branch November 14, 2017 20:54
MylesBorins pushed a commit that referenced this pull request 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 MylesBorins mentioned this pull request Dec 12, 2017
@gibfahn
Copy link
Member

gibfahn commented Dec 13, 2017

Do we want to backport this? I feel like the discussion about what to do with remark hasn't quite finished, so maybe we should hold off. On the other hand we've already backported some of the remark-cli changes to v8.x, so maybe we need to bring this back too.

MylesBorins pushed a commit that referenced this pull request 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 pull request 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>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
rvagg pushed a commit that referenced this pull request 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
meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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