Skip to content

tools: ensure doc-only doesn't update package-lock#21015

Closed
MylesBorins wants to merge 1 commit intonodejs:masterfrom
MylesBorins:fix-npm-install-for-docs
Closed

tools: ensure doc-only doesn't update package-lock#21015
MylesBorins wants to merge 1 commit intonodejs:masterfrom
MylesBorins:fix-npm-install-for-docs

Conversation

@MylesBorins
Copy link
Contributor

@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 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 commented May 29, 2018

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

@MylesBorins
Copy link
Contributor Author

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

MylesBorins commented Jun 1, 2018

@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
@MylesBorins
tools: ensure doc-only doesn't update package-lock
9ed2f01 
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 June 6, 2018 08:50
@MylesBorins
Copy link
Contributor Author

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

MylesBorins commented Jun 6, 2018

landed in 1aa582a

@MylesBorins MylesBorins closed this Jun 6, 2018
MylesBorins added a commit that referenced this pull request Jun 6, 2018
@MylesBorins
tools: ensure doc-only doesn't update package-lock
1aa582a 
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 pull request Jun 6, 2018
@MylesBorins
tools: ensure doc-only doesn't update package-lock
440e899 
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
Merged
MylesBorins added a commit that referenced this pull request Jun 25, 2018
@MylesBorins
tools: ensure doc-only doesn't update package-lock
aea0fbe 
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 pushed a commit to v8/node that referenced this pull request Jul 5, 2018
@MylesBorins @hashseed
tools: ensure doc-only doesn't update package-lock
b08694d 
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
Merged
rvagg pushed a commit that referenced this pull request Aug 16, 2018
@MylesBorins @rvagg
tools: ensure doc-only doesn't update package-lock
79919a3 
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

Reviewers

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@trivikr trivikr trivikr approved these changes

+2 more reviewers

@refack refack refack approved these changes

@devsnek devsnek devsnek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@MylesBorins @Trott @refack @jasnell @lpinca @devsnek @BridgeAR @trivikr @nodejs-github-bot