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: ensure doc-only doesn't update package-lock #21015

Closed

Conversation

MylesBorins
Copy link
Contributor

Currently make doc-only is updating the package-lock.json
which is breaking our release build.

This adds the flags --no-package-lock and --no-audit when
running npm install as part of the make doc-only job.

This is blocking to 10.3.0 so I would appreciate a fast track

@MylesBorins MylesBorins added the fast-track PRs that do not need to wait for 48 hours to land. label May 29, 2018
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label May 29, 2018
@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

Kicked off a test build to ensure docs are working should be available at https://nodejs.org/download/test/v10.3.0-test19ffd1846c/

@Trott
Copy link
Member

Trott commented May 29, 2018

Might the correct solution be #20970?

It seems like we should update package-lock.json when it needs updating. make doc-only is updating it because there's missing entries.

@Trott
Copy link
Member

Trott commented May 29, 2018

#20970 just landed so you should be able to cherry-pick 148b8ad and that should hopefully resolve the problem.

@refack
Copy link
Contributor

refack commented May 29, 2018

I agree that in general make targets should not change any git tracked files.
Maybe even run npm ci instead of install (also possible at some later PR).

@Trott
Copy link
Member

Trott commented May 31, 2018

(Just to be clear, my comments were questions, not objections.)

CI: https://ci.nodejs.org/job/node-test-pull-request/15175/

@MylesBorins
Copy link
Contributor Author

@refack admin?

@jasnell
Copy link
Member

jasnell commented Jun 1, 2018

Not sure what's up with @refack's account here but the exact same comment has been posted to nearly every open pr

@MylesBorins MylesBorins force-pushed the fix-npm-install-for-docs branch 2 times, most recently from c032462 to df734bc Compare June 6, 2018 08:49
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make
@MylesBorins
Copy link
Contributor Author

I've gone ahead and removed --no-audit as it is a useful signal and won't cause CI to fail. Will land later today if there are no objections

@MylesBorins
Copy link
Contributor Author

landed in 1aa582a

@MylesBorins MylesBorins closed this Jun 6, 2018
MylesBorins added a commit that referenced this pull request Jun 6, 2018
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make

PR-URL: #21015
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
MylesBorins added a commit that referenced this pull request Jun 6, 2018
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make

PR-URL: #21015
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jun 6, 2018
MylesBorins added a commit that referenced this pull request Jun 25, 2018
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make

PR-URL: #21015
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
hashseed pushed a commit to v8/node that referenced this pull request Jul 5, 2018
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make

PR-URL: nodejs#21015
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
rvagg pushed a commit that referenced this pull request Aug 16, 2018
Currently `make doc-only` is updating the package-lock.json
which is breaking our release build.

This adds the flags `--no-package-lock` when
running `npm install` to ensure the package-lock.json is not
changed unintentionally by running make

PR-URL: #21015
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
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. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants