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

Implement reminder if using run/run-script without 'install' #10043

Closed
wants to merge 4 commits into from
Closed

Implement reminder if using run/run-script without 'install' #10043

wants to merge 4 commits into from

Conversation

timkrins
Copy link
Contributor

For #10032

Implements warning if using run/run-script with locally defined dependencies but without a node modules directory.

Need help writing tests - couldn't figure out how to achieve a pass/fail through using npm in tap.

kenany
kenany reviewed Oct 21, 2015
View reviewed changes
lib/run-script.js
@@ -3,6 +3,7 @@ module.exports = runScript
var lifecycle = require('./utils/lifecycle.js')
var npm = require('./npm.js')
var path = require('path')
var fs = require('fs')
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well use graceful-fs I think.

@kenany
Copy link
Contributor

kenany commented Oct 21, 2015

I wonder if this is best done here so that this warning only prints if a script actually failed to run.

Also would:

fs.stat(path.resolve(pkgdir, 'node_modules/'), ...)

be "better" than having to do fs.readdir then the indexOf check?

@othiym23
Copy link
Contributor

othiym23 commented Nov 2, 2015

@kenany Agreed. @timkrins if you can make that change, this will get merged shortly thereafter. Thanks!

@timkrins
Copy link
Contributor Author

timkrins commented Nov 2, 2015

Will get onto it tomorrow, cheers!
On 2 Nov 2015 11:19 p.m., "Forrest L Norvell" notifications@github.com
wrote:

@kenany https://github.com/KenanY Agreed. @timkrins
https://github.com/timkrins if you can make that change, this will get
merged shortly thereafter. Thanks!

—
Reply to this email directly or view it on GitHub
#10043 (comment).

@iarna iarna added this to the next-next milestone Jan 6, 2016
zkat pushed a commit that referenced this pull request Jan 14, 2016
@timkrins @zkat
run-script: Warn if using script without install
787ba2c 
It might be helpful to remind people that they're trying to run
a script before they actually run `npm install` on the package.

PR-URL: #10043
Credit: @timkrins
@kenany kenany mentioned this pull request Jan 16, 2016
Closed
iarna pushed a commit that referenced this pull request Jan 21, 2016
@timkrins @iarna
run-script: Warn if using script without install
3109303 
It might be helpful to remind people that they're trying to run
a script before they actually run `npm install` on the package.

PR-URL: #10043
Credit: @timkrins
@iarna
Copy link
Contributor

iarna commented Jan 25, 2016

This landed in 3.6.0! =)

@iarna iarna closed this Jan 25, 2016
This was referenced Jan 31, 2016
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Feb 7, 2016
Closed
Closed
Closed
@tamzinblake
Copy link
Contributor

@timkrins I just encountered this, great work, very helpful!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
merge-to-latest
Projects
None yet
Milestone
3.6.0 & 2.14.16
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@timkrins @kenany @othiym23 @iarna @tamzinblake