★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21015Nouvelle recherche | Portail wanayoo
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
Member

@MylesBorins MylesBorins commented May 29, 2018

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
Copy link
Member Author

@MylesBorins MylesBorins commented May 29, 2018

@MylesBorins
Copy link
Member Author

@MylesBorins MylesBorins commented May 29, 2018

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 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 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
refack approved these changes May 29, 2018
@refack
Copy link
Member

@refack 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).

lpinca
lpinca approved these changes May 29, 2018
@Trott
Copy link
Member

@Trott 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
Member Author

@MylesBorins MylesBorins commented Jun 1, 2018

@refack admin?

@jasnell
Copy link
Member

@jasnell 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 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
@MylesBorins MylesBorins force-pushed the fix-npm-install-for-docs branch from df734bc to 9ed2f01 Compare Jun 6, 2018
@MylesBorins
Copy link
Member Author

@MylesBorins MylesBorins commented Jun 6, 2018

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
Member Author

@MylesBorins MylesBorins commented Jun 6, 2018

landed in 1aa582a

@MylesBorins MylesBorins closed this Jun 6, 2018
MylesBorins added a commit that referenced this issue 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 <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins added a commit that referenced this issue 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 <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jun 6, 2018
MylesBorins added a commit that referenced this issue 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 <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
hashseed added a commit to v8/node that referenced this issue 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 <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
rvagg added a commit that referenced this issue 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 <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants