ES6, new way to extend YO generator #161

maustand · 2018-03-13T08:05:11Z

This commit has a lots of changes..

I have updated all the dependencies again.
I changed all the generator code to es6 ( changing var to let and consts )
I put eslint and the last but one , i have updated the way that you create yeoman generators because you was using YO 0.24.1 and i updated for the last one..

Please you can review the code the code and do the merge...

Important:

Tests are not working, even running npm test in a fresh clone..

I am trying to fix it... meanwhile you can take a look :

Regards

…package.json that it would be generate for the this generator. This dependencies are working perfect in the project that the generator creates. .ignore: i added a few files that it should be ignored. Note: The next dependencies are breaking the tests and/or it would have to do a few changes in the templetes for using it because of the new versions have a new approach for use it. In this way, dont update the next dependencies : babel-jest jest-cli mockgoose

- eslint added to the main project. eslint config file added and removed from the package.json - main project es6 implemented ( changed vars for consts, let and es6 literal objects and functions ) - Main project tests and generator template were broken because the new version of Yeoman has a different way to extend the generator and spawnCommand was moved to another path. - blueBird promises removed.. Now we are going to use js native promises. Note: Main project tests have been fixed but output tests are broken. Probably it is happening because the dependecies update ( new way to use them, ect .)

diegohaz

Thank you very much, @maustand. That's an awesome work.

I made some suggestions on code.

Most of the errors on tests seems to be related to mockgoose. Also, [email protected] has broken some of the mongoose plugins we are using, see diegohaz/mongoose-keywords#12. I think they've fixed it on v5.0.10. Not sure.

diegohaz · 2018-03-13T22:09:27Z

generators/api/index.js

+const recast = require('recast');
+const reservedWords = require('reserved-words');
+
+module.exports = class extends generator {


Since generator is a class, let's make it capitalized: Generator

diegohaz · 2018-03-13T22:11:52Z

generators/api/index.js

+ this.props.lower  = lowerCase(this.props.camel);
+ this.props.lowers  = lowerCase(this.props.camels);
+ this.props.start  = upperFirst(this.props.lower);
+ this.props.starts  = upperFirst(this.props.lowers);


Does that pass eslint? I don't think we need those alignments.

we can create our custom rules...
Currently i am using eslint recommend, but we could use another rules about indentation, spaces, ect..

feel free to modify it.

diegohaz · 2018-03-13T22:13:00Z

generators/api/index.js

-  }) : [];
+ this.props.modelFields.split(',').map((field) => {
+ return field.trim();
+ }) : [];


Shorten:
this.props.modelFields.split(',').map(field => field.trim())

diegohaz · 2018-03-13T22:14:02Z

generators/api/index.js

+ const copyTpl = this.fs.copyTpl.bind(this.fs);
+ const tPath = this.templatePath.bind(this);
+ const dPath = this.destinationPath.bind(this);
+ const filepath = (filename) => {
 return path.join(props.dir, props.kebab, filename);
 };


I think we can make it shorter too: filename => path.join(...)

diegohaz · 2018-03-13T22:18:51Z

generators/app/index.js

-module.exports = yeoman.Base.extend({
- prompting: function () {
- var that = this;
+module.exports = class extends generator { 


Generator

I changed this.. probably i forgot to do it in this file...

diegohaz · 2018-03-13T22:19:29Z

generators/app/index.js

@@ -147,15 +160,15 @@ module.exports = yeoman.Base.extend({
 tPath('api/password-reset'),
 dPath(props.srcDir + '/' + props.apiDir + '/password-reset'),
 props
- );
+  );


Is that space needed?

maustand · 2018-03-14T09:12:39Z

@diegohaz In general i did not modify the code in order to avoid problems.
just i have changed the code in cases that it was impossible to avoid it, just like Yeoman.. becasue they changed the way to extend the generator..

By the way, we should find a way to have the dependencies of the output in a only one place..
Currently we have to update the dependencies of the generator and output as well...
So i updated the generator but i did not do it in the package.json of the output...

So i should change this files in my fork or it is automatic ??

let me know.

diegohaz · 2018-03-14T14:53:41Z

@maustand It's not automatic. You should make the changes in your fork.

The package.json problem is really annoying. I'll try to think in another solution. In the meanwhile, we'll still need to update both package.json.

diegohaz · 2018-03-23T23:22:46Z

@maustand, I fixed tests on the branch https://github.com/diegohaz/rest/tree/maustand-master (tried to push to your repo, but I guess you've created the PR without allowing changes, so I don't have access. In this case, you need to merge https://github.com/diegohaz/rest.git maustand-master into your local branch and push it).

I needed to downgrade Mongoose and Mockgoose. It seems to not be an easy migration, so let's try to upgrade them alter in another PR.

And, for some reason, coverage reports are empty (for the generator). I'm not sure why. Maybe upgrading some gulp plugin caused this.

…to fixes

coveralls · 2018-03-25T11:46:10Z

Coverage decreased (-100.0%) to 0.0% when pulling efab27d on maustand:master into c190f5d on diegohaz:master.

Mauricio Stand and others added 5 commits October 24, 2017 19:10

Merge branch 'master' of git://github.com/diegohaz/rest

48cd24f

Merge remote-tracking branch 'upstream/master'

d42d788

Merge remote-tracking branch 'upstream/master'

a7c082e

diegohaz reviewed Mar 13, 2018

View reviewed changes

Mauricio Stand and others added 4 commits March 14, 2018 18:01

Code Review: reference 2ab4e39

15721b8

Merge remote-tracking branch 'upstream/master'

90bbde5

Downgrade mongoose and mockgoose

5ba3067

Make travis run only Node v7

22bb1a3

Merge branch 'maustand-master' of https://github.com/diegohaz/rest in…

efab27d

…to fixes

pedro-victor mentioned this pull request May 15, 2018

Update project dependencies #174

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ES6, new way to extend YO generator #161

ES6, new way to extend YO generator #161

maustand commented Mar 13, 2018

diegohaz left a comment

diegohaz Mar 13, 2018

diegohaz Mar 13, 2018

maustand Mar 14, 2018 •

edited

diegohaz Mar 13, 2018

diegohaz Mar 13, 2018

diegohaz Mar 13, 2018

maustand Mar 14, 2018

diegohaz Mar 13, 2018

maustand Mar 14, 2018

maustand commented Mar 14, 2018 •

edited

diegohaz commented Mar 14, 2018 •

edited

diegohaz commented Mar 23, 2018 •

edited

coveralls commented Mar 25, 2018

ES6, new way to extend YO generator #161

Are you sure you want to change the base?

ES6, new way to extend YO generator #161

Conversation

maustand commented Mar 13, 2018

diegohaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maustand Mar 14, 2018 • edited

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

Choose a reason for hiding this comment

maustand commented Mar 14, 2018 • edited

diegohaz commented Mar 14, 2018 • edited

diegohaz commented Mar 23, 2018 • edited

coveralls commented Mar 25, 2018

maustand Mar 14, 2018 •

edited

maustand commented Mar 14, 2018 •

edited

diegohaz commented Mar 14, 2018 •

edited

diegohaz commented Mar 23, 2018 •

edited