radiegtya / meteoris2

a Realtime Javascript Boilerplate base on Meteor Js Framework
MIT License
246 stars 46 forks source link

Generate CRUDSS to package directory and install immediately #66

Closed martinhbramwell closed 9 years ago

martinhbramwell commented 9 years ago

Please test this thoroughly.

radiegtya commented 9 years ago

can you explain in simple way about this? so many change

martinhbramwell commented 9 years ago

https://github.com/warehouseman/meteoris/blob/master/README.md#whats-new-in-v09806

martinhbramwell commented 9 years ago

Please install my fork in a temporary directory and give it a try. Use the CRUDSS generator while observing the packages directory. If you create a CRUDSS page for "jobs" in a namespace "hr" then you will see a directory package/jobs. The file packages/jobs/package.js contains the definition for a package fully identified as hr:jobs. That reference is appended to .meteor/packages and can be removed by typing meteor remove hr:jobs.

radiegtya commented 9 years ago

then it will not modifiedable? I don't think package is a good solution though, how if we have 200 collection inside our apps? we'll have 200 package? Do you think it will be better if only the crudss generator / mugen ROLE generator which are being packages?

martinhbramwell commented 9 years ago

Packages are just as easy to modify as code anywhere else in your app. 200 packages are dramatically easier to manage than 200 collections managed by scattered files. Meteor Developer's Group strongly recommends classifying ALL application code into packages.

In my opinion, having the CRUDSS generator integrated into Meteoris is a damaging anti-pattern. It should be extracted to its own package as soon as possible. A package able to create new packages in any application, not just Meteoris, is a powerful concept , don't you think?

Please experiment with what I have done so far. In a few days I will deliver Mugen modified to generate fully internationalized CRUDSS packages.

radiegtya commented 9 years ago

tomorrow I'll try this. can you give me the link, that MDG reccomends code into packages? because I think packages just a place for helpers, and not the main code.

martinhbramwell commented 9 years ago

I cannot look for the direct reference right now.

I have seen many references to the importance of packages. This one is the most succinct and detailed : BUILDING LARGE SCALE METEOR APPS http://meteor.redandivory.com/#/

jeangui commented 9 years ago

hi all,

very interesting presentation.

cheers jeangui

martinhbramwell commented 9 years ago

Another article about packages as an architectural best-practice : Meteor project structure — the way forward

jeangui commented 9 years ago

thanks jeangui

radiegtya commented 9 years ago

HI @martinhbramwell , I've tested it. Here are my review: 1.The main problem for this, is at namespace, I'll consider using this if the namespace are really used, not only for the package name (like composer for example). For example if I make namespace like this "radiegtya.inventory" and I have collection name "products" and "movements". the folder structure should be like this:

packages
├── radiegtya
        └── inventory
                └── products
                └── movements 
I'll give example for movements naming convention

-  Naming convention in views template, should be like this:
   radiegtya_inventory_movementsIndex, radiegtya_inventory_movements_form etc
-  Naming convention in controllers, should be like this:
   Radiegtya.Inventory.MovementsController
-  Naming convention in collections, should like this:
   Radiegtya.Inventory.Movements
-  Naming convention in server, should like this:
   Radiegtya.Inventory.MovementsServer

nb: the filename no need to change, it was already correct

because if the apps getting larger and larger, your package will make another problem in the future.

2.Package dependency for related collection. I think if we have relation in our apps, we should automatically adding setting for dependency for the packages in the package.js setting.

How do you think?

martinhbramwell commented 9 years ago

Namespace nesting, as you suggest, is controversial.

I did look into doing it, but it is more difficult, and not clearly necessary at all. The main reason I did not do it is because Meteor does not do it. They do not have a concept of a hierarchy of packages, and I have seen no mechanisms to support it.

The controversy hinges on whether it is necessary to define structures that follow the apparent namespace hierarchy versus simply grouping by name.

If the latter is all that is required then your example would be as simple as :

 packages
 ├── radiegtya-inventory-products
 ├── radiegtya-inventory-movements

And the naming within the files .meteor/packages and .meteor/versions would look like this :

 radiegtya:inventory-products
 radiegtya:inventory-movements
radiegtya commented 9 years ago

That's because I am coming from PHP Symfony, Yii, n Laravel. So I am just habit to use their naming convention. But the packages name you recommended is ok though. "radiegtya-inventory-products" but how do you think about this point?

I'll give example for movements naming convention

-  Naming convention in views template, should be like this:
   radiegtya_inventory_movementsIndex, radiegtya_inventory_movements_form etc
-  Naming convention in controllers, should be like this:
   Radiegtya.Inventory.MovementsController
-  Naming convention in collections, should like this:
   Radiegtya.Inventory.Movements
-  Naming convention in server, should like this:
   Radiegtya.Inventory.MovementsServer

nb: the filename no need to change, it was already correct

and also this point

Package dependency for related collection. I think if we have relation in our apps, we should automatically adding setting for dependency for the packages in the package.js setting.

martinhbramwell commented 9 years ago

Yes, I think that naming convention is fine. I also agree that it will be necessary to be smart about dependencies between packages.

I have opened this Pull Request as a way to initiate discussion of these changes. It is not ready to be pulled however. Right now, it is a work in progress that will require another week, I think, to get cleaned up and ready for inclusion in your master copy. I am doing the work because it is important for the app I need to create. If you are aware of other imminent Pull Requests please advise their authors to watch this thread and please advise me before accepting any PRs, as the burden of merging will then be on me and may be quite difficult at this point.

Later this week I will be committing more changes, including full i18n of generated packages :

I have had a little bit of difficulty with TAP.i18n :

Other stuff :

Please be patient, but expect a testable version in the next few days.

radiegtya commented 9 years ago

Ok I now, the merging process will be very hard if I merge another PR. So before that, I'll wait your PR. Then do you mind also make the private/mugen/views template files with i18n too? So almost all thing will be translated.

Thanks @martinhbramwell

martinhbramwell commented 9 years ago

I have generated a package using Mugen, which I am altering for I18N, ensuring that I have all the translation keys done correctly, and that everything still works properly. When that is done I will make a backup copy of the package, and generate exactly the same package again. The diffs between the two will show me where to apply changes to your templates. Also, I will need a new template for the English I18N file.

martinhbramwell commented 9 years ago

@radiegtya Ega. Please peruse my latest commit. You will see I have added a bit more to the i18n work.

https://github.com/warehouseman/meteoris/commit/a87ca5514baddb6fcbb519437866540236896ab5

Please be sure to take the time to create a full Bahasa Indonesia version as soon as you possibly can! It's only fair after the work I am doing on my end, right?

martinhbramwell commented 9 years ago

I did a further commit last night.

I believe there are no more raw text strings anywhere; I have converted all of them to I18n look-up keys. Mugen now generates an i18n directory into the package directory of each newly generate collection, with standardized lookup pairs for most things. (I think really there should be an I18n package dedicated to the CRUDSS key/value pairs that don't have replacement variables. There should not be duplicates of those ones.)

You will see that I also added I18n of dates/times using add on packages for MomentJS.

The i18next package is very smart. I did not go very far with exploiting all its capabilities but I did use it for plural vs singular. So ...

en_I18n.json

"role_action":"Role Action",
"role_action_plural":"Role Actions",

mugen/client/role/views/mugenRoleActions/index.html

{{_ "role_action" count=0}}  //  gives Role Actions
{{_ "role_action" count=1}}  //  gives Role Action
{{_ "role_action" count=2}}  //  gives Role Actions
{{_ "role_action" count=99}}  //  gives Role Actions

Finally, I am addressing a bug in I18n of Moment right now Shouldn't the package momentjs:moment be Tracker reactive?

martinhbramwell commented 9 years ago

Ok. Got the Moment problem fixed. Added a French version. CRUDS now emit with i18n for en, es, fr and psuedo zh-CN.

radiegtya commented 9 years ago

Sorry for long vacum @martinhbramwell.

It's ok that you will merge so many code to this meteoris repo. Because I had already backed up the meteoris repo (I named it first stable version) at https://github.com/radiegtya/meteoris-v1.

maybe it will make this many code rebuild being meteoris v2

martinhbramwell commented 9 years ago

Thank you for taking the time to do that.

"maybe it will make this many code rebuild being meteoris v2" I don't really understand what you mean by that.

radiegtya commented 9 years ago

@martinhbramwell : I mean this repo is freely to be always merged with your code, and it might be future of meteoris v2. So your code merging are very welcome and always merged, as soon as I had free time, I would also help.

btw how about the modular package thing? I am really waiting for that feature comes true

martinhbramwell commented 9 years ago

"how about the modular package thing? I am really waiting for that feature comes true"

You really should thoroughly test what I have done!

This PR "Generate CRUDSS to package directory and install immediately." IS "the modular package thing", as you call it. It allows you to generate a CRUDSS collection as a distinct Meteor package. You can build a library of different CRUDSS packages. You can then instantiate a new Meteoris instance, and install one (or several) of the CRUDSS packages from the library simply by referring to it (them) in the instance's .meteor/packages file.

If it does not work please let me know.

radiegtya commented 9 years ago

ok I'll test it this afternoon. Please wait my report. @martinhbramwell

radiegtya commented 9 years ago

@martinhbramwell : I've tested it, and sorry for the late progress.

[1]. The first problem here was, if you create two same collection with different namespace using mugen generator. It still generates same collection.

for example:

martinhbramwell:someCollections
radiegtya:someCollections

it will make applications ERROR, because:

SomeCollections = new Mongo.Collection("someCollections"); //they are generating same collection name

I think you should make better namespacing like:

Martinhbramwell.SomeCollections = new Mongo.Collection("someCollections");
//and
Radiegtya.SomeCollections = new Mongo.Collection("someCollections");

this will also need changing on views, controllers, and server.

btw this meteoris version also have error when generating camel case collection, for example: accountingJournals (I think I had been fixing this, but idk why meteoris having this error again)


[2]. The second problem (which can also answer the first problem) was, in mugen it will better if we have meteor-username, namespace, and collection fields. not only namespace and collection fields.

for example, if I want to generate module I can fill the form like this

meteoris-username: radiegtya
namespace: accounting
collection: journals

so it will provide package name => radiegtya:accounting-journals
and the collection name => Radiegtya.Accounting.Journals
mongo collection name => accounting-journals
server name => Radiegtya.Accounting.JournalsServer
controllers name => Radiegtya.Accounting.JournalsController
template name => radiegtya-accounting-journalsIndex, radiegtya-accounting-journalsView etc

[3]. Third, meteoris and mugen should have its own package so it will easily installed on another projects and it will minimalize the folder structure (lib, client, and server on root will be gone completely).


[4]. Package dependencies on mugen generator. if you are relating one collection to other, it should make dependency to another related package which is generated using meteoris.


How do you think @martinhbramwell :)

martinhbramwell commented 9 years ago

Points : 1 & 2

Basing package naming on user names is not a good idea. Employees come and go. No company, and certainly no client, wants packages named after the guy who authored the first version.

There are too many different use cases for it to make sense to enforce any particular scheme. See : Namespaced Collections in Meteor

Point 3 : Yes, This is certainly the next step. Is stopped where I did to wait for feedback.

Point 4 : Agreed.

radiegtya commented 9 years ago

Point 1 & 2: you can simply named it with organization name. because meteor require developer to add their/organization name in front of package name.

and about the collection name. for example, I had organization/name: piyiku package name: accounting collection name: journals

the collection code will be:

Piyiku.Accounting.Journals = new Meteor.Collection('accounting.journals'); //the mongo collection given name are without the organization/person name

//OR

Piyiku.Accounting.Journals = new Meteor.Collection('accounting-journals');
martinhbramwell commented 9 years ago

I do understand that an org name is required on the package. However it is a structural issue of Meteor framework that should not be allowed to dictate solutions to the customer's problems.

The correct solution to the case you mention is :

It should be possible to do it that way right now.

radiegtya commented 9 years ago

@martinhbramwell : no it can't please test it again and make something like that. the "-" cannot be type for collection name

radiegtya commented 9 years ago

@martinhbramwell : I had been experimental making meteoris with my employee as standalone package. but there are some problem because it. I had been write it here https://trello.com/b/01SvtPLA/meteoris-roadmap. Ofc I'll fix this ASAP. Don't worry, I had been updating the README.md and consider this meteoris version as meteoris version 2. Thanks

martinhbramwell commented 9 years ago

I'm not sure I understand. By, "standalone package" do you mean -- such that a package created with Mugen can be used in any other Meteor project without Meteoris? This would be a very good thing, imo.

radiegtya commented 9 years ago

@martinhbramwell have you seen the new README.md? Please take a look and try latest meteoris update which is simply "meteor add meteoris:meteoris-core" in your new meteor project. Further detail in README.md. And the core code was in this repo "packages/meteoris-core".

btw about stand alone package I mean was this https://atmospherejs.com/meteoris/meteoris-core

Thanks

martinhbramwell commented 9 years ago

I have been busy with other things, really.

It would be very good to upgrade the documentation at this point. Would you like to do a quick preliminary job on it, without paying attention to the quality of language? I do not have the time to go through it in detail identifying all that has changed, but since you have been making changes very recently you will easily spot alterations and additions. Just identify and specify what needs to be said and I offer to do the actual writing. Sound good?

radiegtya commented 9 years ago

It's ok, you already helps me improve Meteoris. Maybe in a near time better I'll make tutorial in video format. Then You can simply take a look at it, and doing the documentation. How about that?

martinhbramwell commented 9 years ago

Actually, I think we should collaborate on the video. :-)

martinhbramwell commented 9 years ago

Here's one I did last year ... https://www.youtube.com/watch?v=JFk5LPSx6p8

radiegtya commented 9 years ago

Tomorrow Morning I'll see the video. Btw what do your project right now that make you so busy?

martinhbramwell commented 9 years ago

@radiegtya "Btw what do your project right now that make you so busy?"

https://forums.meteor.com/t/instant-rest-client-from-a-swagger-specification/6259/3