search

saschakiefer / generator-openui5

yeoman generator for OpenUI5 applications and assets
Other
64 stars 17 forks source link

Yeoman generator cleanup #41

Closed js1972 closed 10 years ago

js1972 commented 10 years ago

http://yeoman.io/blog/cleanup.html

js1972 commented 10 years ago

We can use the yeoman config file (class: Storage - http://yeoman.github.io/generator/Storage.html) to store data. We can use this instead of the node-localstorage module to tell the view generator about actions in the app generator amongst other things...

js1972 commented 10 years ago

Refactor of script-base.js to use extend() instead of inherit() and calling the Base Generator. Done.

js1972 commented 10 years ago

Changes for this issue are in the yeoman_cleanup branch.

js1972 commented 10 years ago

One of the yeoman cleanup tasks is to update all dependency requirements. We have a couple that are out of date (or more to the point: newer versions are available). Shall we update these as part of this task to the current verions? QUnit and JSHint are outdated on the generated apps and for the project...

grunt-contrib-jshint (package: ~0.6.4, latest: 0.8.0) mocha (package: ~1.12.0, latest: 1.17.1) load-grunt-tasks (package: ~0.2.1, latest: 0.3.0) grunt-mocha-test (package: ~0.6.3, latest: 0.9.0) marked (package: ~0.2.10, latest: 0.3.1)

saschakiefer commented 10 years ago

node-localstorage: With my refactoring of the classical application I removed that dependency completely, since I implicitely generate a view. But the Storage is definitively an option, when we need sty. similar again in the future.

extend(): Did you change that also in the generators, which extend script base? If not, I can do that tonight if you want.

Regarding the updates: :+1:

js1972 commented 10 years ago

I'm struggling with the cleanup tasks... I changed the index.js and script-base.js to use the new concept but I can't get access to the basic details prompts in script-base.js. All the other prompts are fine except for these ones. I don't understand the runtime of the generator well enough to get to the bottom of it yet. What I do notice is that I think the yeoman.io team have gotten ahead of themselves and sent out their cleanup notification a little early as none of the yeoman.io managed generators (webapp, angular, etc) are even using this technique so I'm thinking I'm going to revert and kill the yeoman_cleanup branch.

The only useful things are: a) We can use the yeoman config file for passing information to sub-generators (if required) instead of node-localstorage (this may not be necessary if you've removed the need to call the sub-generator on app scaffolding) b) we can use the "david" tool to automatically update our dependencies in package.json.

So... I'll kill my cleanup branch then create a new one with just the dependency updates... what do you think? Unless you want to take a look at my branch - I'm not sure that the refactor gives us anything. It looks like in the future yeoman is moving toward an architecture where you can use different generators on your app, etc..

On 4 February 2014 14:57, saschakiefer notifications@github.com wrote:

node-localstorage: With my refactoring of the classical application I removed that dependency completely, since I implicitely generate a view. But the Storage is definitively an option, when we need sty. similar again in the future.

extend(): Did you change that also in the generators, which extend script base? If not, I can do that tonight if you want.

Regarding the updates: [image: :+1:]

Reply to this email directly or view it on GitHubhttps://github.com/saschakiefer/generator-openui5/issues/41#issuecomment-34035264 .

saschakiefer commented 10 years ago

You could create a new branch weigh the working refactoring changes and I could have a look at this one tonight maybe I find sth (didn't have a chance to look at the new yeoman features yet). If I don't find anything either, we just remove it and wait for the examples.

js1972 commented 10 years ago

Its doing weird things... I think its best just to kill of my branch. I really need examples and yeoman have provided not one yet. as I mentioned this refactor was nothing more than a chore - its does not make the code any simpler.

I don't want to waste your time so will: 1) blow away my yeoman_cleanup branch 2) create a new branch with just the node module version updates then pull-request.

Then we can discuss what the next things to do are... Maybe we should push for a stable release version now to coincide with the blog.

On 4 February 2014 15:09, saschakiefer notifications@github.com wrote:

You could create a new branch weigh the working refactoring changes and I could have a look at this one tonight maybe I find sth (didn't have a chance to look at the new yeoman features yet). If I don't find anything either, we just remove it and wait for the examples.

Reply to this email directly or view it on GitHubhttps://github.com/saschakiefer/generator-openui5/issues/41#issuecomment-34035725 .

saschakiefer commented 10 years ago

:8ball: ;-) OK, fine with me

js1972 commented 10 years ago

Problem: I've just killed off yeoman_cleanup branch and re-run the generator from before and the same issue is there - the package.json does not get the variables substituted in using template(). There is something wrong with the prompts gathered in script-base.js - promptForBasicDetails(). All other prompts looks to be working fine. This issue occurs for all generators. Its got something to do with splitting out the prompting into the script-base.js file.

I'll raise this a a new bug issue.

js1972 commented 10 years ago

Closing this issue as we'll wait for examples from other generators on refactoring to use the extend modules instead of util.inherits