Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Document new shrinkwrap dev dep behavior #14519

Closed
wants to merge 1 commit into from
Closed

Document new shrinkwrap dev dep behavior #14519

wants to merge 1 commit into from

Conversation

iarna
Copy link
Contributor

@iarna iarna commented Nov 3, 2016

Back in 4.0.1 I made shrinkwraps include devepment dependencies by default, but forgot to update the associated documentation.

Fixes: #14479

@iarna iarna added this to the next milestone Nov 3, 2016
othiym23
othiym23 approved these changes Nov 3, 2016
View reviewed changes
Copy link
Contributor

@othiym23 othiym23 left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM.

doc/cli/npm-shrinkwrap.md
`npm install --only=production` they won't be installed. Similarly, if
the environment variable `NODE_ENV` is `production` then they won't be
installed. You can request they not be included with:
`npm shrinkwrap --production` or `npm shrinkwrap --only=prod`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about flipping the order of these around. It would be nice to (eventually) deprecate --prod and --dev and only use the clearer --only and --also in their place.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Flipping the order in both places.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

iarna
iarna commented Nov 3, 2016
View reviewed changes
doc/cli/npm-shrinkwrap.md
installed `devDependencies` are excluded, then npm will print a
warning. If you want them to be installed with your module by
default, please consider adding them to `dependencies` instead.
Starting with npm v4.0.0, `devDependencies` are included when you run
Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/4.0.0/4.0.1/

othiym23
othiym23 approved these changes Nov 3, 2016
View reviewed changes
Copy link
Contributor

@othiym23 othiym23 left a comment

Choose a reason for hiding this comment

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

LGTM 🐑🚀🔥

iarna added a commit that referenced this pull request Nov 3, 2016
@iarna
doc: Document new shrinkwrap dev dep behavior
7f41295 
Fixes: #14479
Credit: @iarna
PR-URL: #14519
Reviewed-By: @othiym23
@iarna
Copy link
Contributor Author

iarna commented Nov 3, 2016

merged as 7f41295

@iarna iarna closed this Nov 3, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62561cd on iarna/shrinkwrap-docs into * on release-next*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62561cd on iarna/shrinkwrap-docs into * on release-next*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62561cd on iarna/shrinkwrap-docs into * on release-next*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62561cd on iarna/shrinkwrap-docs into * on release-next*.

@legodude17
Copy link
Contributor

@coveralls is being really unhelpful.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62561cd on iarna/shrinkwrap-docs into * on release-next*.

@iarna iarna deleted the iarna/shrinkwrap-docs branch December 1, 2016 00:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@othiym23 othiym23 othiym23 approved these changes

Assignees
No one assigned
Labels
review target-latest
Projects
None yet
Milestone
4.0.2 & 3.10.10
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@iarna @coveralls @legodude17 @othiym23