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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

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

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

@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
@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
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
@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 <[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
@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 <[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
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 <[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
@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 <[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
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 <[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
Reviewers

@trivikr trivikr trivikr approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@devsnek devsnek devsnek approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

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