Closed UndeadBaneGitHub closed 8 years ago
LGTM
LGTM, other than the 2 above comments?
I'm having trouble using the mean-module from this PR. I pulled down this branch, and linked it to my npm; and I've verified that the link worked as shown in my screen shot below.
It appears that the references to the selected options are undefined for me. For instance, this.serverFolders.addControllersFolder
returns undefined when I write it to the console. However, the props
and this.serverFolders
look correct to me. What I don't undestand is how the check for this.serverFolders.addControllersFolder
would return true. How does that reference work?
I chose not to create the models directory, to illustrate that the props
were getting set correctly.
I should note:
NPM: 2.11.3
Node: 0.12.7
Windows 7
@mleanos Ok, I see what you mean. Figuring out the problem now.
LGTM. I've tested, and confirm this is working as expected. One point of confusion at first is why no files were created (other than the client-side & server-side config files); maybe put a message that might point the user to using the individual generators.
Was there a specific reason why we don't want to create any of the files when creating a module with mean-module
? This is merely for clarification, as it doesn't affect this PR at all. Also, I tested the express-routes
generator & that worked as well.
@UndeadBaneGitHub Thank you for your hard work on this. It's all looking very nice!
@mleanos Well, this mean-module
generator is almost direct derivative of an old angular-module
generator - it only creates folders structure and essential files, nothing more.
For the "more" part, there will be a more advanced (and complex) CRUD generator, as it used to be, which has not been implemented for this version. I wanted to first implement all the "parts" of it, all the controllers/models/routes generators. Now that all the tools are set, it actually can be done - and I will try to address it shortly.
My pleasure! :)
SGTM. Thanks, again! I'll be looking forward to future enhancements :)
@UndeadBaneGitHub @mleanos just waiting on the timeout change right? Then squash and we should be good.
@ilanbiala Added this.timeout(0)
back to the test suites and squashed
I also tested this PR on both Windows and Linux machines. Looks ok currently.