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.

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

yo yo, may i help you? #36

Merged
merged 1 commit into from Jun 28, 2013

Conversation

Projects
None yet
5 participants
@stephenplusplus
Contributor

stephenplusplus commented Jun 4, 2013

Here's our man, yo, being all helpful and stuff.

(#34)

Just a start! Anyone who has time to take a look and go, "Whoa dude, no," please do :)

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jun 4, 2013

Member

Awesome work! 👍

Member

passy commented Jun 4, 2013

Awesome work! 👍

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Jun 5, 2013

Member

very cool.

Member

paulirish commented Jun 5, 2013

very cool.

Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
@addyosmani

This comment has been minimized.

Show comment
Hide comment
@addyosmani

addyosmani Jun 5, 2013

Member

Great work once again, @stephenplusplus :) I think this would be a perfect fit for 1.1.

Member

addyosmani commented Jun 5, 2013

Great work once again, @stephenplusplus :) I think this would be a perfect fit for 1.1.

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 5, 2013

Contributor

I realized there's more to the isaacs.iriscouch.com thing, so it now actually searches and returns multiple results :)

Contributor

stephenplusplus commented Jun 5, 2013

I realized there's more to the isaacs.iriscouch.com thing, so it now actually searches and returns multiple results :)

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 10, 2013

Member

Should we maybe put it in a new file? yo file is getting long.

Member

sindresorhus commented Jun 10, 2013

Should we maybe put it in a new file? yo file is getting long.

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 10, 2013

Contributor

The yo yo stuff is in yoyo.js, next to the yo file itself.

Contributor

stephenplusplus commented Jun 10, 2013

The yo yo stuff is in yoyo.js, next to the yo file itself.

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 10, 2013

Member

Nvm mind me...

Member

sindresorhus commented Jun 10, 2013

Nvm mind me...

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 26, 2013

Member

I think we should merge this right into yo (not and not use yo yo). Maybe even also let users interactively select which generator to run. Wdyt?

Member

sindresorhus commented Jun 26, 2013

I think we should merge this right into yo (not and not use yo yo). Maybe even also let users interactively select which generator to run. Wdyt?

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus
Member

sindresorhus commented Jun 26, 2013

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jun 26, 2013

Member

+1 on running generators directly through yo yo. I guess the fact that we can't expose cli flags like --coffee doesn't really matter here, does it?

Member

passy commented Jun 26, 2013

+1 on running generators directly through yo yo. I guess the fact that we can't expose cli flags like --coffee doesn't really matter here, does it?

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 26, 2013

Member

@passy no, it's just for a quick convenience.

Member

sindresorhus commented Jun 26, 2013

@passy no, it's just for a quick convenience.

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 26, 2013

Contributor

Updated to just run on yo. It also lists:

Yeoman is a mask worn by the following members of the open-source community:

  Paul Irish, Addy Osmani, Mickael Daniel, Sindre Sorhus, Eric Bidelman,
  Frederick Ros, Brian Ford, Pascal Hartig, Stephen Sawchuk, and countless
  other contributors.

(open to other ideas for how to word that)

Beneath the welcome message:

[?] What would you like to do? (Use arrow keys)
  [X] Generate a new Angular application
  [ ] Generate a new Ember application
  [ ] Update your generators
  [ ] Install a generator
  [ ] Find some help
  [ ] Get me out of here!
Contributor

stephenplusplus commented Jun 26, 2013

Updated to just run on yo. It also lists:

Yeoman is a mask worn by the following members of the open-source community:

  Paul Irish, Addy Osmani, Mickael Daniel, Sindre Sorhus, Eric Bidelman,
  Frederick Ros, Brian Ford, Pascal Hartig, Stephen Sawchuk, and countless
  other contributors.

(open to other ideas for how to word that)

Beneath the welcome message:

[?] What would you like to do? (Use arrow keys)
  [X] Generate a new Angular application
  [ ] Generate a new Ember application
  [ ] Update your generators
  [ ] Install a generator
  [ ] Find some help
  [ ] Get me out of here!
@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 26, 2013

Member

Generate a new Angular application

Not all the generators generates applications. Maybe word it like: "Run the Angular generator" ?

Member

sindresorhus commented Jun 26, 2013

Generate a new Angular application

Not all the generators generates applications. Maybe word it like: "Run the Angular generator" ?

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 26, 2013

Contributor

Good call @sindresorhus. Updated.

Contributor

stephenplusplus commented Jun 26, 2013

Good call @sindresorhus. Updated.

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jun 26, 2013

Member

Could you rebase this? I probably broke this with the sudo-block update. 😬

Member

passy commented Jun 26, 2013

Could you rebase this? I probably broke this with the sudo-block update. 😬

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 26, 2013

Contributor

Done @passy.

Contributor

stephenplusplus commented Jun 26, 2013

Done @passy.

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jun 26, 2013

Member

Works great for me. This is so rad!

Member

passy commented Jun 26, 2013

Works great for me. This is so rad!

Show outdated Hide outdated bin/yoyo.js
Show outdated Hide outdated bin/yoyo.js
this.home();
};
util.inherits(yoyo, generator.Base);

This comment has been minimized.

@stephenplusplus
@stephenplusplus

This comment has been minimized.

@sindresorhus
@sindresorhus
// Rolls through all of the generators provided by `env.generators`, finding
// their `package.json` files, then storing them internally in `this.pkgs`.
yoyo.prototype._findGenerators = function findGenerators() {

This comment has been minimized.

@sindresorhus

sindresorhus Jun 27, 2013

Member

Why do we have to duplicate the generator search logic. Can't we expose the package in the generator system somehow?

@sindresorhus

sindresorhus Jun 27, 2013

Member

Why do we have to duplicate the generator search logic. Can't we expose the package in the generator system somehow?

This comment has been minimized.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

Good thinking! I'll take a look at that now.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

Good thinking! I'll take a look at that now.

This comment has been minimized.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

@sindresorhus From yeoman-generator, we would have to tack on the same logic in this function (_findGenerators) to find the package.json, then read the file and store it to be accessed internally. After going down that route, there's the benefit of having access to each generator's package.json, but the negative is it would increase the start-up time to allow for finding/reading the JSON files. What do you think?

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

@sindresorhus From yeoman-generator, we would have to tack on the same logic in this function (_findGenerators) to find the package.json, then read the file and store it to be accessed internally. After going down that route, there's the benefit of having access to each generator's package.json, but the negative is it would increase the start-up time to allow for finding/reading the JSON files. What do you think?

This comment has been minimized.

@sindresorhus

sindresorhus Jun 27, 2013

Member

I don't see why it would be slower as we're already doing this ^ on startup?

@sindresorhus

sindresorhus Jun 27, 2013

Member

I don't see why it would be slower as we're already doing this ^ on startup?

This comment has been minimized.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

It's currently looking for index.js:

Globing for generator *:* with pattern +(generator-)*/lib/generators/**/index.js (cwd: /Users/ssawchukii/dev/yo/node_modules)

Unless it's somewhere else package.json is read?

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

It's currently looking for index.js:

Globing for generator *:* with pattern +(generator-)*/lib/generators/**/index.js (cwd: /Users/ssawchukii/dev/yo/node_modules)

Unless it's somewhere else package.json is read?

This comment has been minimized.

@sindresorhus

sindresorhus Jun 27, 2013

Member

Yes, but I don't see how it will be slower to move this method into the generator system?

@sindresorhus

sindresorhus Jun 27, 2013

Member

Yes, but I don't see how it will be slower to move this method into the generator system?

This comment has been minimized.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

Oh, just because it's only the "yo yo" prompts making use of it. Just guessing, but a generator is probably used directly much more often than "yo" alone, so I was just avoiding additional lookups unless necessary.

@stephenplusplus

stephenplusplus Jun 27, 2013

Contributor

Oh, just because it's only the "yo yo" prompts making use of it. Just guessing, but a generator is probably used directly much more often than "yo" alone, so I was just avoiding additional lookups unless necessary.

This comment has been minimized.

@sindresorhus

sindresorhus Jun 27, 2013

Member

good point. nvm then. we can bring it up again if we see any usecase in the future.

@sindresorhus

sindresorhus Jun 27, 2013

Member

good point. nvm then. we can bring it up again if we see any usecase in the future.

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jun 28, 2013

Contributor

Well, I added some tests, but I can't figure out why it's bombing on Travis. It works locally. 📈 📉

Edit: It's bombing because a test helper from yeoman-generator needed a tweak. I've obviously lost my mind.

The yoyo tests will all pass when that's in a release :)

Contributor

stephenplusplus commented Jun 28, 2013

Well, I added some tests, but I can't figure out why it's bombing on Travis. It works locally. 📈 📉

Edit: It's bombing because a test helper from yeoman-generator needed a tweak. I've obviously lost my mind.

The yoyo tests will all pass when that's in a release :)

sindresorhus added a commit that referenced this pull request Jun 28, 2013

@sindresorhus sindresorhus merged commit fa689e7 into yeoman:master Jun 28, 2013

1 check failed

default The Travis CI build failed
Details
@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jun 28, 2013

Member

Landed! This is so nice. I <3 these kinds of usability improvements that makes it easier to use :D

🍰 🍰 🍰

Member

sindresorhus commented Jun 28, 2013

Landed! This is so nice. I <3 these kinds of usability improvements that makes it easier to use :D

🍰 🍰 🍰

@revathskumar revathskumar referenced this pull request Jun 28, 2013

Closed

Issue with yo yo #41

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