这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@jamesplease
Copy link
Member

Resolves #58

  • Differentiate package.jsons
  • Move actual deps from devDeps in generator package.json
  • Ensure all files are copied over
  • Update variables in those files
  • Figure out API for not exporting a global
  • Add generator tests
  • Update Travis test for the generator
  • Fix Travis (//cc @thejameskyle / @megawac )
  • Update README
  • Remove global var option
  • Fix license escaping
  • Update name of initial test

@jamesplease
Copy link
Member Author

@thejameskyle, I can't seem to get travis to run successfully. Any clue? Here's the output from travis.

@jamiebuilds
Copy link
Contributor

Well the actual error is below. The problem is that yeoman is trying to execute npm whoami but travis does not have a user.

@jamiebuilds
Copy link
Contributor

Are we going to rename this repo to generator-babel-library or something?

@jamesplease
Copy link
Member Author

Are we going to rename this repo to generator-babel-library or something?

Yeah, something like that. I might contact the guy who owns generator-babel to see if he would give me that package name.

Well the actual error is below. The problem is that yeoman is trying to execute npm whoami but travis does not have a user.

Hmm. Were you able to get this working in your fork? Maybe I'll add a try/catch around it.

function getDefaultName() {
  var name;
  try {
    name = npm.whoami();
  } catch(e) {}
  return name;
}

@jamiebuilds
Copy link
Contributor

Maybe I'll add a try/catch around it.

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't want to do a loop over files after a fs.readdir()?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would work. I just copy-pasted from James' generator. I don't have a strong opinion tbqh. Do you prefer a loop?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems more flexible - anyway thats what I did IIRC in generator-babel-node


It would be better if it was a recursive readdir (ls -R), however I didn't see a way to do that at the time

Also proposed it here jamiebuilds#8

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 will-change

Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts james?

Copy link
Member Author

Choose a reason for hiding this comment

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

#197 (comment)

tl;dr, yes, but maybe in an iteration.

Copy link
Member Author

Choose a reason for hiding this comment

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

@megawac why not just this.directory('.');?

@megawac
Copy link
Contributor

megawac commented Aug 3, 2015

Other than some of the file names looks good

@megawac
Copy link
Contributor

megawac commented Aug 3, 2015

I was also thinking, would it make sense to add a prompt like project type: node (babel-node-boiler) browser/both (babel-library-boiler) and select scaffolds accordingly. All this would require is two template paths. That way we can merge generator-babel-node and this generator

@megawac
Copy link
Contributor

megawac commented Aug 3, 2015

Well the actual error is below. The problem is that yeoman is trying to execute npm whoami but travis does not have a user.

Hmm. Were you able to get this working in your fork? Maybe I'll add a try/catch around it.

function getDefaultName() {
  var name;
  try {
    name = npm.whoami();
  } catch(e) {}
  return name;
}

I got around this using promises

    Promise.all([gitConfig(), exec('npm whoami').catch(function(e) {
      console.error('Error getting npm user name: run `npm login`');
      console.error(e);
    })])
    .then(function(args) {
      this.config = args[0];
      this.username = trim(args[1][0]);
      this._showPrompts(done);
    }.bind(this));

@jamesplease
Copy link
Member Author

would it make sense to add a prompt like project type

Ya I plan to do that and a few other things in a future iteration. This is just a first version tho'.

@jamesplease
Copy link
Member Author

I got around this using promises

Ah, cool. I think the try-catch is a bit less ugly, so I'm going to try that first.

@jamesplease
Copy link
Member Author

SUCCESS.

thanks for the help @megawac.

@jamesplease jamesplease force-pushed the yeoman branch 4 times, most recently from 6e1ab24 to 9b65d73 Compare August 4, 2015 03:39
@jamesplease
Copy link
Member Author

@megawac I like your simplified copying of files, but I want to stick with the Yeoman conventions for now. I may revisit later. I just want to get this first version merged 4now.

jamesplease added a commit that referenced this pull request Aug 4, 2015
@jamesplease jamesplease merged commit 6f3623b into master Aug 4, 2015
@jamesplease jamesplease deleted the yeoman branch August 4, 2015 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants