Add generator.installDependencies to install bower and npm dependencies #205

Merged
merged 1 commit into from Apr 11, 2013

Conversation

Projects
None yet
5 participants
Member

passy commented Apr 10, 2013

With the new concurrent installation of bower/npm dependencies and the option to skip the automatic installation, the code became a bit too large to be copied around comfortably.

generator.installDependencies accepts three options:

// Options:
//  - npm: bool, whether to run `npm install`, default: true
//  - bower: bool, whether to run `bower install`, defaulkt: true
//  - skipInstall: bool, whether to skip automatic installation and just print a
//                 message to the user how to do it, default: false
Member

passy commented Apr 10, 2013

Will update this to include @kevva's Windows workaround: yeoman/generator-webapp#49

Member

kevva commented Apr 10, 2013

Super nice!

Member

ragingwind commented Apr 10, 2013

good!

Owner

sindresorhus commented Apr 10, 2013

Lol, like minutes after I merged the PR and published a new release of webapp.

Member

passy commented Apr 10, 2013

@sindresorhus Hah, and this is only the start. For every change you request, I'll open at least two new PRs here. :suspect:

Owner

sindresorhus commented Apr 10, 2013

Would be nice if generator.installDependencies() called generator.npmInstall() and generator.bowerInstall() which supported args and stuff. That way we would be able to use be individually and without the message. We already have bowerInstall, so why not an npmInstall one too. Thoughts?

Member

passy commented Apr 10, 2013

@sindresorhus Good point! We probably should put them together in one file. Idea for a name?

Owner

sindresorhus commented Apr 10, 2013

install.js

Member

passy commented Apr 10, 2013

Actually, bowerInstall is currently named just install. I'd like to rename that to bowerInstall. Should we keep install around as an alias with some deprecation warning?

Owner

sindresorhus commented Apr 10, 2013

@passy Go for it. nah, we'll just add it to the changelog.

Owner

addyosmani commented Apr 10, 2013

Amazing! Nice work.

Member

passy commented Apr 11, 2013

There we go:

generator.npmInstall mirrors generator.bowerInstall regarding the arguments. I kept installDependencies as a shortcut that also generates a message for the user.

If you're not feeling adventurous, you should probably wait until tomorrow before reviewing this, because I'm terribly tired right now. ;)

@sindresorhus sindresorhus commented on an outdated diff Apr 11, 2013

changelog.md
@@ -1,3 +1,8 @@
+### 0.xx.x - TBA

@sindresorhus sindresorhus commented on an outdated diff Apr 11, 2013

test/fetch.js
@@ -35,8 +35,8 @@ describe('yeoman.generators.Base', function () {
this.homedir = process.platform === 'win32' ? process.env.USERPROFILE : process.env.HOME;
});
- it('generator.install(name)', function (done) {
- this.dummy.install('backbone', function(err) {
+ it('generator.bowerInstall(name)', function (done) {
+ this.dummy.bowerInstall('backbone', function(err) {
@sindresorhus

sindresorhus Apr 11, 2013

Owner

function (err)

Owner

sindresorhus commented Apr 11, 2013

lgtm.

@passy when this is merged, can you recompile the docs, so they're up to date with the new methods?

@passy passy generator.install
Includes three new methods:

    - generator.npmInstall()
    - generator.bowerInstall() (formerly: generator.install())
    - generator.installDependencies(), which combines the former two
2e02473

@passy passy added a commit that referenced this pull request Apr 11, 2013

@passy passy Merge pull request #205 from passy/installdeps
Add generator.installDependencies to install bower and npm dependencies
a7eb8fb

@passy passy merged commit a7eb8fb into yeoman:master Apr 11, 2013

Member

passy commented Apr 11, 2013

@sindresorhus Will do!

passy deleted the passy:installdeps branch Apr 11, 2013

Member

passy commented Apr 11, 2013

@sindresorhus Um, where do I find "tomdox" to generate the docs. Google only shows me the Gruntfile of this very repository.

Member

passy commented Apr 12, 2013

@sindresorhus Would you mind pushing a new release soon so we can update the generators?

Owner

sindresorhus commented Apr 12, 2013

Done

@passy it throws if cb is not specified:

events.js:130
    throw TypeError('listener must be a function');
          ^
TypeError: listener must be a function
    at TypeError (<anonymous>)
    at ChildProcess.EventEmitter.addListener (events.js:130:11)
    at runInstall (/Users/sindresorhus/Projects/generator-karma/node_modules/yeoman-generator/lib/actions/install.js:63:6)
    at Generator.install [as npmInstall] (/Users/sindresorhus/Projects/generator-karma/node_modules/yeoman-generator/lib/actions/install.js:153:10)
    at Generator.setupEnv (/Users/sindresorhus/Projects/generator-karma/app/index.js:34:10)
    at next (/Users/sindresorhus/Projects/generator-karma/node_modules/yeoman-generator/lib/base.js:275:18)
    at Generator.run (/Users/sindresorhus/Projects/generator-karma/node_modules/yeoman-generator/lib/base.js:289:4)
    at Environment.run (/usr/local/lib/node_modules/yo/node_modules/yeoman-generator/lib/env.js:424:20)
    at init (/usr/local/lib/node_modules/yo/bin/yo:68:7)
    at pre (/usr/local/lib/node_modules/yo/bin/yo:81:3)

peterblazejewicz referenced this pull request in OmniSharp/generator-aspnet Mar 17, 2015

Closed

phase out `--skip-install` option #86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment