Adapt changes of yeoman generator 1.0.0 #170

mischah · 2016-12-03T18:48:02Z

~~Hej, I need help 😖~~

~~I’m stucked with passing the namespace argument when composing with generator:subgenerator.~~

~~See: https://github.com/yeoman/generator-generator/compare/yeoman-generator/1.0.0?expand=1#diff-2018087f584c4398b5c3a23fc0e5f9dbL63~~

~~I’m confused.~~

All set and done

🎉

Needed to bump to prevent side effects with generator-node

SBoudrias · 2016-12-03T21:53:09Z

app/index.js

    });

-    this.composeWith('generator:subgenerator', {
+    this.composeWith(require.resolve('../subgenerator'), {
      arguments: ['app']


This needs to be the name of the argument you want to pass in. It's treated the same as an option now.

{namespace: 'app'}

Thanks. Tried that too. Doesn’t work as well :(
Pushed that for you one minute ago.

Oops. Missed one line. It does work.

SBoudrias · 2016-12-03T21:54:42Z

subgenerator/index.js

-    generators.Base.apply(this, arguments);
+module.exports = class extends Generator {
+  constructor(args, opts) {
+    super(args, opts);

    this.argument('namespace', {
      type: String,
      required: true,
      description: 'Generator namespace'


Old error, but that key as always been desc instead of the full description.

Using `namespace` leads to `this.options.namespace === 'generator:subgenerator'` instead of `this.options.namespace === 'app'`

coveralls · 2016-12-03T23:16:27Z

Coverage increased (+0.3%) to 97.619% when pulling 02b7f29 on yeoman-generator/1.0.0 into e40c828 on master.

coveralls · 2016-12-03T23:18:55Z

Coverage remained the same at 97.368% when pulling 02b7f29 on yeoman-generator/1.0.0 into e40c828 on master.

mischah · 2016-12-03T23:20:44Z

Passing the arguments works now. Thanks @SBoudrias

But I had to rename the argument.
Using namespace leads to this.options.namespace === 'generator:subgenerator' instead of this.options.namespace === 'app'

I guess because of the parameter of composeWith named namespace ?

… to work with yeoman-generator v1.0.0

coveralls · 2016-12-03T23:43:45Z

Coverage remained the same at 97.368% when pulling cba5e01 on yeoman-generator/1.0.0 into e40c828 on master.

SBoudrias · 2016-12-03T23:57:25Z

.travis.yml

@@ -1,6 +1,7 @@
 sudo: false
 language: node_js
 node_js:
-  - stable
+  - 7


node is the name for the latest stable version. I think I prefer this to 7 given it won't go into LTS.

SBoudrias · 2016-12-03T23:59:13Z

subgenerator/index.js


-    this.argument('namespace', {
+    this.option('name', {


This can stay an argument. The issue is that namespace was a "reserved" name for yeoman-environment.

Mhm. Using this.argument('name', { leads to failing test.

Tests are failing with:

1) generator:app defaults "before all" hook: Error: Did not provide required argument name! at .<anonymous> (node_modules/yeoman-generator/lib/index.js:335:33) at Array.forEach (native) at Base.checkRequiredArgs (node_modules/yeoman-generator/lib/index.js:331:19) at Base.parseOptions (node_modules/yeoman-generator/lib/index.js:316:8) at argument (node_modules/yeoman-generator/lib/index.js:246:8) at new <anonymous> (subgenerator/index.js:9:447) at Environment.instantiate (node_modules/yeoman-environment/lib/environment.js:336:10) at composeWith (node_modules/yeoman-generator/lib/index.js:485:26) at default (app/index.js:9:2522) at Object.<anonymous> (node_modules/yeoman-generator/lib/index.js:387:23) 2) generator:subgenerator "before all" hook: Uncaught Error: Did not provide required argument name! at .<anonymous> (node_modules/yeoman-generator/lib/index.js:335:33) at Array.forEach (native) at Base.checkRequiredArgs (node_modules/yeoman-generator/lib/index.js:331:19) at Base.parseOptions (node_modules/yeoman-generator/lib/index.js:316:8) at argument (node_modules/yeoman-generator/lib/index.js:246:8) at new <anonymous> (subgenerator/index.js:9:447) at Environment.instantiate (node_modules/yeoman-environment/lib/environment.js:336:10) at Environment.create (node_modules/yeoman-environment/lib/environment.js:313:15) at RunContext._run (node_modules/yeoman-test/lib/run-context.js:90:29) at RunContext.<anonymous> (node_modules/yeoman-test/lib/run-context.js:58:10)

That could be a bug. I'll investigate!

Actually that's pretty simple. It's breaking because generator-node is pre 1.0, so it doesn't recognize the arguments being passed as part of the options.

We should fix that in yeoman-generator so we keep compatibility with older generators too.

Do we have to change something over here?

mischah · 2016-12-04T00:00:29Z

Guess I’m ready to review now.

About failing tests:
They are green locally.

On travis it says:

1) generator:app defaults "before all" hook:
     Uncaught Error: No ESLint configuration found.

I guess that is because the latest version of generator-node/generators/eslint stores the ESLint settings in the package.json?

SBoudrias · 2016-12-04T00:06:26Z

Also, could we copy the packages version from our own package.json? I think that'd simplify version bumping as we'd always match generator-generator version with the version it generates.

mischah · 2016-12-04T13:37:31Z

How can we get the content of the package.json of generator-generator?

I only have nasty ideas like regexing this.resolved which is in my case:

'/Users/mkuehnel/Documents/Projects/generator-generator/app/index.js'

from the package.json of generator-generator

mischah · 2016-12-04T15:44:34Z

test/app.js

-          'yeoman-generator': '^0.23.0',
-          chalk: '^1.0.0',
-          yosay: '^1.0.0'
+          'yeoman-generator': 'yeoman/generator#bump-version-for-updating-generator-generator',


Need to change this when v1.0.0 is released.

We should update the tests to also read from package.json like the generator

mischah · 2016-12-04T15:44:53Z

package.json

-    "yeoman-generator": "^0.23.3",
-    "yosay": "^1.0.2"
+    "superb": "^1.3.0",
+    "yeoman-generator": "yeoman/generator#bump-version-for-updating-generator-generator",


Need to change this when v1.0.0 is released.

mischah · 2016-12-04T19:00:04Z

The failing tests are caused by a bug of gulp-eslint adametry/gulp-eslint#186 😖

Because of adametry/gulp-eslint#186

shinnn · 2016-12-04T20:28:51Z

bug of gulp-eslint

~~No.~~ (edit) Maybe, no.

SBoudrias · 2016-12-04T21:17:04Z

app/index.js

@@ -62,15 +62,17 @@ module.exports = class extends Generator {

  writing() {
    var pkg = this.fs.readJSON(this.destinationPath('package.json'), {});
+    var generatorGeneratorPkg = this.fs.readJSON(path.normalize(this.resolved.replace(/app\/index.js/, 'package.json')));


you can just use require('../package.json')

🙈 Thanks and done.

SBoudrias · 2016-12-04T21:18:02Z

.travis.yml

@@ -2,5 +2,6 @@ sudo: false
 language: node_js
 node_js:
  - stable
+  - 6
  - 5


Let's drop 5 and replace stable by node.

mischah · 2016-12-04T21:27:25Z

One last to do on my site:
ES6ify generator-generator

mischah · 2016-12-04T21:33:51Z

Question:
What do do think about dropping the validation: https://github.com/yeoman/generator-generator/pull/170/files#diff-2018087f584c4398b5c3a23fc0e5f9dbL26

validate: function (str) {
  return str.length > 'generator-'.length;
}

It’s dead code because of:

function makeGeneratorName(name) {
  name = _.kebabCase(name);
  name = name.indexOf('generator-') === 0 ? name : 'generator-' + name;
  return name;
}

The generated generator isn’t ES6ified for now.

mischah · 2016-12-04T22:11:33Z

generator-generator is successfully ES6ified 🎉

The generated generator is still using ES5 syntax.
And I don’t want to change this for now because of weird Linting errors on Travis CI 😕

mischah · 2016-12-04T23:59:57Z

Please remember to squash that mess when merging 🙈

SBoudrias · 2016-12-08T06:12:56Z

Hey, you can use 1.0.0-rc1 and move back to using arguments.

Until generator-node goes to 1.0, you'll need pass the arguments as before with {arguments: ['foo']}

mischah · 2016-12-08T23:23:26Z

Hey, you can use 1.0.0-rc1 and move back to using arguments.

Done

Until generator-node goes to 1.0, you'll need pass the arguments as before with {arguments: ['foo']}

Done. That’s slightly confusing 😆

SBoudrias · 2016-12-09T06:44:04Z

@mischah code looks good! How do you feel about the API changes overall?

mischah · 2016-12-09T09:47:47Z

@SBoudrias I like them a lot. Especially the new composeWith signature is much more readable and easier to understand 💖

mischah added 7 commits December 2, 2016 16:07

Update dependencies

b159a7b

Get latest yeoman-generator from GitHub for now

932533f

Update app

3ca95f4

[WIP] Adapt breaking changes of yeoman-generator 1.0

a372ee5

Enable ES6 for ESLint

f92896d

Reference branch bump-version-for-updating-generator-generator

ab6885f

Needed to bump to prevent side effects with generator-node

Fix requiring the subgenerator

6fdcf4e

mischah force-pushed the yeoman-generator/1.0.0 branch from 484b4c6 to 6fdcf4e Compare December 3, 2016 21:00

SBoudrias reviewed Dec 3, 2016

View reviewed changes

Fix passing the namespace

f66b24b

mischah force-pushed the yeoman-generator/1.0.0 branch from 965bd76 to f66b24b Compare December 3, 2016 22:46

mischah added 2 commits December 4, 2016 00:13

Fix generating the generator

a9fc6c5

Using `namespace` leads to `this.options.namespace === 'generator:subgenerator'` instead of `this.options.namespace === 'app'`

Remove logging

02b7f29

mischah added 3 commits December 4, 2016 00:29

Update the generated generator …

58d66d3

… to work with yeoman-generator v1.0.0

Test node 6 + 7 on travis

5b44b91

Fix test

cba5e01

mischah changed the title ~~[WIP] Adapt changes of yeoman generator 1.0.0~~ Adapt changes of yeoman generator 1.0.0 Dec 3, 2016

SBoudrias requested changes Dec 3, 2016

View reviewed changes

Update travis config

7635e1b

Dynamically get dependencies for the generated generator …

0990016

from the package.json of generator-generator

mischah commented Dec 4, 2016

View reviewed changes

Rename .eslintrc to make tests green again

51fb745

mischah force-pushed the yeoman-generator/1.0.0 branch from 4a76c5a to 3071aa1 Compare December 4, 2016 18:42

Downgrade gulp-eslint

249963f

Because of adametry/gulp-eslint#186

mischah force-pushed the yeoman-generator/1.0.0 branch from e47decc to 249963f Compare December 4, 2016 19:59

ES5 extending for the generated generator for now

d187e66

shinnn mentioned this pull request Dec 4, 2016

Using extends in ESlint settings causes an error adametry/gulp-eslint#186

Closed

SBoudrias reviewed Dec 4, 2016

View reviewed changes

mischah added 2 commits December 4, 2016 22:25

Less hacky way to read the package.json of generator-generator

f496680

Update travis config

ca0a257

ES6ify generator-generator

e7abcd8

The generated generator isn’t ES6ified for now.

mischah force-pushed the yeoman-generator/1.0.0 branch from 6a8469e to e7abcd8 Compare December 4, 2016 22:03

SBoudrias approved these changes Dec 4, 2016

View reviewed changes

Use ^1.0.0-rc1 of yeoman-generator

cf7aee5

Change handling argument for generator name

2ff0754

mischah force-pushed the yeoman-generator/1.0.0 branch from ab06347 to 2ff0754 Compare December 8, 2016 23:24

SBoudrias merged commit 640bdc5 into master Dec 9, 2016

SBoudrias deleted the yeoman-generator/1.0.0 branch December 9, 2016 19:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt changes of yeoman generator 1.0.0 #170

Adapt changes of yeoman generator 1.0.0 #170

mischah commented Dec 3, 2016 •

edited

Loading

SBoudrias Dec 3, 2016

mischah Dec 3, 2016

mischah Dec 3, 2016

SBoudrias Dec 3, 2016

coveralls commented Dec 3, 2016

coveralls commented Dec 3, 2016

mischah commented Dec 3, 2016

coveralls commented Dec 3, 2016

SBoudrias Dec 3, 2016

SBoudrias Dec 3, 2016

mischah Dec 4, 2016

SBoudrias Dec 4, 2016

SBoudrias Dec 4, 2016

mischah Dec 4, 2016

mischah commented Dec 4, 2016

SBoudrias commented Dec 4, 2016

mischah commented Dec 4, 2016

mischah Dec 4, 2016

SBoudrias Dec 4, 2016

mischah Dec 4, 2016

mischah commented Dec 4, 2016 •

edited

Loading

shinnn commented Dec 4, 2016 •

edited

Loading

SBoudrias Dec 4, 2016

mischah Dec 4, 2016

SBoudrias Dec 4, 2016

mischah Dec 4, 2016

mischah commented Dec 4, 2016

mischah commented Dec 4, 2016

mischah commented Dec 4, 2016 •

edited

Loading

mischah commented Dec 4, 2016

SBoudrias commented Dec 8, 2016

mischah commented Dec 8, 2016

SBoudrias commented Dec 9, 2016

mischah commented Dec 9, 2016

Adapt changes of yeoman generator 1.0.0 #170

Adapt changes of yeoman generator 1.0.0 #170

Conversation

mischah commented Dec 3, 2016 • edited Loading

All set and done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 3, 2016

coveralls commented Dec 3, 2016

mischah commented Dec 3, 2016

coveralls commented Dec 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mischah commented Dec 4, 2016

SBoudrias commented Dec 4, 2016

mischah commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mischah commented Dec 4, 2016 • edited Loading

shinnn commented Dec 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mischah commented Dec 4, 2016

mischah commented Dec 4, 2016

mischah commented Dec 4, 2016 • edited Loading

mischah commented Dec 4, 2016

SBoudrias commented Dec 8, 2016

mischah commented Dec 8, 2016

SBoudrias commented Dec 9, 2016

mischah commented Dec 9, 2016

mischah commented Dec 3, 2016 •

edited

Loading

mischah commented Dec 4, 2016 •

edited

Loading

shinnn commented Dec 4, 2016 •

edited

Loading

mischah commented Dec 4, 2016 •

edited

Loading