Too much stuff in /lib/locomotive . Move to a locomotive-core gem?

[kikito]

That folder is full of stuff. I would not mind it very much if it wasn't for the fact that I can use Spork to test changes on it, so I'm very slow on making changes to Locomotive using TDD.

Wouldn't it make sense to move all that code to a separate gem, with its own tests?

[did]

Hi Enrique, which version of locomotive / ruby do you use ? Because, since I cleaned my gems / my imac and switched to ruby 1.9.3, the application on my machine launches more quickly. Do not forget that locomotive relies on many gems. Last thing, I'm not sure if it would change something to move that code into a gem. Or perhaps, we can extract some functionalities. What do you suggest exactly ?

[kikito]

Hi Didier, thanks for answering

I have not updated to 1.9.3 yet - I'm in 1.9.2. My machine is a decent desktop beast - PC with Ubuntu, quad core, plenty of memory.

I'm using a fairly decent snapshot of locomotive - maybe 1 month old. The problem I have is that (for the /lib/ folder only) I can't use spork - it doesn't "update" the changes on the lib folder; I think this is the expected behavior. As a result, every test run has to load all rails, which takes minutes. Doing TDD this way is very slow.

What do you suggest exactly ?

I suggest moving as much code as possible out of /lib/. Having a lot of code there is a bad sign; I'd put it in the same place as having lots of code in a controller. When someone has lots of code in a controller, the usual strategy is "move that code to a model". When someone has lots of code in the lib/ folder, the usual comment is "move it to a different gem".

You are probably more capable of answering the question of how to do this than myself, but here is my attempt:

Take all code in /lib/ that can be tested in isolation (without Rails), and move it to a gem, along with its tests. Any code that depends on rails should be either refactorized so it's a bit more isolated, or moved back to app/, so it can be properly reloaded during tests (maybe having an app/controllers/locomotive, app/models/locomotive etc).

Regards!

[did]

First, you should install ruby 1.9.3, it solves the slowness when you start a rails app.

You're probably right about /lib. A lot of the code are in there just for patching a couple of gems (for instance, add a to_liquid method to some objects, ...etc). On the other side, I talked to Paul (@paulsponagl) and he told that as a developer, he liked that locomotive was not split into a billion of gems (like for example in spree). Let's talk about that on skype when you'll have time.

[jipiboily]

In fact, Spree and Refinery are split in multiple gems, but you can use only "spree" which itself include all the other gems. That will let you use only some modules and not the others. Especially auth is a problem currently. It is not possible to use Spree and Locomotive together as there is a Devise version difference.

If Locomotive have used Devise 2, or better, had auth just like Forem, I would have love to use it in my current project which involves a Spree store.

You might want to take a read on Ryan Bigg's blog about engines and auth: http://ryanbigg.com/2012/03/engines-and-authentication/

My 2 cents! ;)

[did]

thanks for the feedback @jipiboily. Actually, one of my friends sent me this link this morning. Very interesting and I also left a comment :-)

Concerning a deep relationship between Spree and LocomotiveCMS, well, I don't know what to think about. At first, I was exciting by the idea of integrating other engines with LocomotiveCMS. But let's face the truth, that's a pure fantasy. And that's not a matter of having or not the auth in a separate module. It's more than that. LocomotiveCMS comes with its own UI, its own way of dealing with models and so on. And I even don't address the possible conflicts between their gems which is already a nightmare within LocomotiveCMS itself.

Like I said in my comment, LocomotiveCMS as well as Spree (but I may be wrong) are kind of frameworks. What I mean by that is that you build applications on top of those engines and not the opposite.

Again, I don't pretend to know everything and I may be wrong about the distinction I make between frameworks and engines. LocomotiveCMS and Spree are definitively 2 frameworks or in other words 2 "master" engines.

I'd like to hear the thoughts of the other developers. @mariovisic ? @paulsponagl (because he tried to integrate spree and locomotivecms) ?

[paulsponagl]

i had spree and locomotive running in one app - with a few changes to the Gemfiles, and at first i thought this is a good idea.

But i realized that this will not make much sense.

1) "Supporting" engines are good for integrating functionality into an app - but bad for integrating a User Interface. 2) Authorization has to be part of the main app, or the "Base" Engine. 3) If you need consistent I18n and Multi Tenancy you are in fact lost for now.

My choice now is to use locomotive as my "Base Framework" and to write the e-commerce stuff as an engine for locomotive. (or even core if did allows me to ;) Locomotive has everything a good "core app" should have.

1) Nice administrative UI. 2) Multi Tenancy. 3) Deeply integrated Internationalization via Mongoid. 4) Flexible UI-designable Models. 5) A huge plus within locomotive is using Mongodb/Mongoid as a base datastore - you do not have to write tons of migrations - wanna use an additional field - use it! wanna use a new model/collection - add it!

A word to spree:

i.m.O. the bunch of engines within it (auth, core, promo, dash, api ...) are not necessary. Who would/could use promo, dash, api as a standalone gem ? Nobody !

And if additional engines need to interfere with the User-Interface they have to hook in with deface - nice possibility indeed, but what for - to be prepared for core updates?

In fact the defacing and decorating within spree gems is nothing more than a cleaner way to write patches for the core. And a patch is a patch - it can and will break sometimes. If the patches are small in terms of functional and interface usage - it's ok. But if you want to add more stuff - like multi domain/tenancy - it becomes ugly.

And thats the other problem with engines used the spree way - one tends to the saying "keep the core small". And "write a 'patching' engine for it."

This was not the recipe for success when Linux grew up...

[kikito]

Hi again Did,

Let's talk about that on skype when you'll have time.

Please don't misunderstand me - I'm not willing to be rude, but I think I have said all I could on this matter. Nevertheless, I'll be sending you my details via gh.

I have digged up some references regarding the lib folder.

This post details how you can put more than just models inside app/models :

http://lindsaar.net/2008/5/17/tip-18-take-back-your-app-folder

This is an old question (it mentions the "plugins" directory instead of gems) but the answers are still valid: by externalizing the code in lib/ to a separate gem, you increase modularity, etc (also, tests are faster).

http://stackoverflow.com/questions/7637266/why-make-a-rails-plugin-instead-of-putting-code-in-lib

[did]

hey Enrique, my friend ! don't be sorry ! I'm happy you brought this topic up :-) I do think you are making a good point, improving the readability of the code is always a good thing !

Basically, I can see 4 kinds of code in the lib folder

1/ improving classes: like helpers for the action_controller class

2/ patching some gems (mongoid, carrierwave, httparty, ...etc)

3/ custom drops/filters/tags for Liquid

4/ the logic for the rendering

3/ could be in another gem for sure. Probably 4/ too. For the two first, I'm not quite sure if it makes sense to move into an external gem.

[jipiboily]

To be honest, I don't think it is pure fantasy. Sure, engines are still young and there isn't a lot ot them yet. I am also aware that it not on only a matter of having auth separated. I blogged about mountable engines vs dependencies: http://jipiboily.com/2012/rails-mountable-engines-dependency-nightmare.

I was at Spreeconf a few weeks ago and the guys from Spree were talking about an idea (no code as of now) of a common admin system where your could hook your engine. You then could have the same admin for Spree, Refinery and Forem. That's what I understood of the idea. This is only conceptual for now, I know! :)

Still, without a common admin, it is better to have a CMS and Spree together than not having or the CMS or the e-commerce part. Even worse if you need to build one the two because they can't live together. I think is is all the magic and beauty of engines! Spree has some basic CMS extensions, but too basic for our needs. A CMS is not enough either for our e-commerce needs. We need both, and we could at some time need something else, all in the same app! We don't want to reinvent the wheel by coding a part or the other but put our efforts in the customization.

I do see Spree as an e-commerce framework more than a platform and use it that way. That said, I see no reason as to why I couldn't use Locomotive and Spree together (conceptually I mean, as currently it is not possible. I use Refinery for now.). We always have very custom projects, so we need to have custom content type for what we could call the static pages, and we also need a customized shopping experience.

In the end, you'll decide what direction Locomotive takes: it's all up to you! :) But I think engines should reduce dependencies to it's bare minimum so we could, and will, want to use multiple engines within a single project.

Again, my 2 cents! :)

[did]

Thanks for your precious feedback @paulsponagl :-)

[jipiboily]

@paulsponagl don't you think that it'll be a big overhead to write the e-commerce part? Think of the cart, checkout process, users with multiple shipping/billing adresses, payment gateways, taxonomies, variants, taxes (!), etc.

[did]

great post @jipiboily.

However, I've got a couple of questions about your refinery / spree integration.

• Do your clients bother about having 2 different kind of UI (one for the CMS part, the other one for the e-commerce) ?
• If you have the same UI for the back-office of your app, which engine UI did you tweak ?
• If I separate the auth from the core engine, will it solve your problems ?

One more thing, I think spree should not deal with the CMS functionality at all and should leave it to others (refinery, locomotive, radiant, ...etc) thru "connectors". It's an idea, not sure if it makes sense.

Jean-Philippe, that debate is very important for the future of the engines. It's still a wild world and we need to define / write conventions. (btw thank you so much for sharing your experience and your thoughts)

[jipiboily]

We are using both admin as they ship. I kinda monkey patched for now. You can find that code here: https://github.com/jipiboily/spree_refinery_auth

I use the Spree::Role and Spree:User models for both Spree and Refinery. It works. It's not what we could talk a great integration at that level, but I can still work with both.

Separating the auth from the core would be a major step forward I think, but there might be other dependency issues. I won't learn you anything but each engines as a lot of dependencies and there might be other dependency problems. That said, decoupling your gem in multiple gems can have that advantage too: you only use the parts you need! Doing that, it also reduce dependencies.

I also think Refinery should not deal with CMS stuff, and I would add that CMS should not deal with e-commerce either. A common admin would be awesome!

I started to work on what might become a mountable blog engine and I want to use as little dependency as possible so it can be easily integrated in bigger projects without problems. No Devise, no Kaminari or WillPaginate, etc...that means more work on the engine, but I think it will be better on the long run. That said, I might be wrong! ;)

[paulsponagl]

@jipiboily: before i knew locomotive i did it exactly the same with refinery (auth part spree, cms part refinery). But after i have seen locomotive i knew that this is the cms of my choice :)

regarding the "wheel reinventing" ;)

i reinvented this wheel three times during my webwork - first perl/mod_perl, second php, third rails. I think i got the concepts that are needed to do this (in an european market)

cart, checkout process => in fact this is not too havy stuff i.m.O. (incl. admin orders with state-machine)

users with multiple shipping/billing adresses => My last rails portal has this already - i think spree is discussing about it.

payment gateways => there is a lot of stuff out there -> and the spree extensions e.g. paypal seem to be unfinished too... (btw. no engine is needed - you need a good core library as a seperate gem with a well defined API - and integrate this into the e-commerce core - i think payment gateways are core features for an e-commerce suite)

taxonomies => finding/classifing products is, i.m.E. a matter of a) a good text based search and b) a tree two levels deep to browse. -> yes, locomotive should have a good text based search - i thought of giving sphinx a try (but need to discuss this with didier) - the rest can be part of an cms too...

variants -> if you look at a mongodb document you can solve everything around this with an embedded 2-dim structure variants, discounts, volume_prices to name a few - (you need two! more engines in spree to solve this - this should be core i.m.O.)

taxes => i will write this for our european customers - we have a simple tax system based on countries, not states -> i think someone out there will be able to care about taxes for the us :)

[jipiboily]

@paulsponagl you're probably right, this is not that much work. There is a lot of librairies to use for payment and everything.

That said, no offense, but for my part, I prefer to use Spree and a CMS and put my efforts on something else. :)

[paulsponagl]

@jipiboily: no offense too - it was not my goal to involve you from day zero :) - but time will tell ;)

[kikito]

Hi guys,

Don't take offence on this, but I think this talk of integration with Spree is Off-Topic for the context of this issue. I created this issue because of the amount of code inside /lib/ ; that's all.

@jipiboily : If you want to discuss about Spree any further, kindly create a separate issue.

Now, going back to the theme at hand:

1/ improving classes: like helpers for the action_controller class 2/ patching some gems (mongoid, carrierwave, httparty, ...etc) 3/ custom drops/filters/tags for Liquid 4/ the logic for the rendering

1/ I'd rather have that inside the app/ directory - as mentioned before, this helps with reloads on tests. 2/ I think the most common approach in that case is forking the gems, applying the patch on the fork, and pointing the Gemfile to that particular gem folder. Another approach is creating a separate gem ("carrierwave_locomotive_patch") and include both the gem and the gem patch on the gemfile. 3/ This is the only thing I'm not sure about. My instinct tells me "create a separate gem called locomotive_liquid_extensions", but I'm not familiar enough with the Liquid internals to say this with confidence. It might not be possible to do so. 4/ The rendering logic is precisely the thing that surprised me the most. Why isn't it put inside the page model and the render controller/helpers?

[mariovisic]

hi @kikito

The classes are not failing to reload because they are in /lib but rather because the configuration for the rails app / spork is not correct to reload on each request. You can add the lib path to be reloaded each time you have any changes by altering the application.rb on this line https://github.com/locomotivecms/engine/blob/master/config/application.rb#L19

Check the spork docs to ensure that any changes to lib get reloaded as well (that might not be necessary after the first change).

Moving the /lib folder to a separate gem will increase reusability. Though the problem is that there are not any circumstances were you would want to use the lib and not the rest of locomotive I suspect. Therefore it will just separate locomotive into 2 chunks which may be beneficial to some extent but harder to maintain and develop.

[kikito]

@mariovisic :

RE: spork . I confess I have only used Spork in a couple places but I remember having tried making Spork load the /lib/ folder without success in the past. But maybe that was a rails 2.x issue that doesn't apply to 3.x (it was some time ago). Will give it a try. Would you accept a patch request that includes lib in Spork?

RE: maintaniability: I understand and agree with your arguments regarding the moving things out of /lib/ to a separate gem. Splitting the code into several parts has advantages but also downsides. So we might as well leave it in one place. You have convinced me of that much.

I wonder what is your opinion about moving things from /lib/ to /app/ though. The rendering code, for example. Why is it on /lib/ instead of in the render controller and page model (maybe with a presenter )?

[did]

RE: maintaniability: ah ah ! Enrique, you had convinced me to move the code about Liquid to another gem so that I can re-use it in the editor without having to maintain the same code at 2 different places :-)

The rendering code is pretty "complex" as you can see in the lib/locomotive/render.rb file. That being said, perhaps, we could move a part of that code into an extension of the model Page (the part about retrieving a page). Besides, the code relative to build the output response should not be in the controller in my point of view because I want to keep my controllers as light as possible. I'm not sure a presenter would change something unless I miss something (which is possible :-) ). What would you use a presenter for ?

[kikito]

Hi did,

ah ah ! Enrique, you had convinced me to move the code about Liquid to another gem so that I can re-use it in the editor without having to maintain the same code at 2 different places :-)

Ha! I didn't think about that. Yeah, reusing a core functionality between the editor and engine is certainly a big advantage. Now I've changed my opinion again; two gains (modularity and reusability in two projects) outweight one disadvantage (code management is more complex).

Besides, the code relative to build the output response should not be in the controller in my point of view because I want to keep my controllers as light as possible

I respect you a lot. But I don't believe that the current Render controller is "light". Sure, it has very few lines of code. But it includes Locomotive::Render, which has lots of lines of code. That's not really light. A metaphor: that's what happens in the movie Shallow Hal. The girl looked thin ... until the guy tried to put his arms around her.

What people mean when they say "fat model, thin controller" is not that the "amount of lines of code in the controller file should be small". They mean that the controller should do as little as possible. Right now, the render controller has few lines of code, but it still is responsible for lots of stuff.

What would you use a presenter for ?

A presenter is usually a place where you put the logic that you need "merely for views". It's not exactly business logic, so it doesn't make sense to include it in the model. But at the same time, depends heavily on the model attributes, and you might want to use it in several views and controllers, so helpers are kindof not appropiate. The railscast I linked makes a better job at explaining what it is.

But it was just an idea. My point is that the rendering logic is "interesting", and right now the object responsible for that it is a controller (since the render logic is included). Moving as much "interesting" logic as possible "really out" of the controller is a good idea.

[paulsponagl]

hi kikito, did, mario,jipiboily,

i thought about seperating the liquid stuff into its own gem. It was at first a 'bad feeling in my stomach' but now i think i have an idea whats not so good about that.

A gem on github should (of course everything now i.m.O. - no offence please) be an entity that follows a few rules of thumb:

1 - it should have a nearly 100% test coverage. 2 - it should not have a circular dependency to another gem. 3 - it should be useable by more than one public project.

at 1) is clear i.m.O. if one is not able to test the gem on it's own it should be part of the projects that use this functionality to achiev the test coverage - this points to at 2) gem a should not rely on gem b and vice versa. this seems to be a design flaw, as the maintainance can be difficult when things break. at 3) if one want to have lots of contributors to a project things have to be kept simple. Every additional git clone that is necessary makes things more complex.

i submitted a feature to the liquid stuff yesterday. i had to make changes to the page model too. it would not ease the situation if i had to submit two pull requests for it...

again, no offence - just a few thoughts ...

best paul

[paulsponagl]

btw. i haven't taken a deeper look to it, but http://book.git-scm.com/5_submodules.html could be another possibility to solve things like this ...

[mariovisic]

Thanks everyone for the great discussion.

For the moment we're still busy trying to get all the bugs fixed for the 2.0 release. Any major changes to the gem infrastructure will not take place until the 3.0 release or later.

I think splitting up locomotive into subgems would be beneficial, although we're already starting to do that to some degree already. We already have the locomotive-heroku rather than having heroku compatibility at the core, we have locomotive liquid, custom_fields etc....

I'm going to close this issue for the moment. The last comment is from a couple of months ago anyways. if anyone has any more to say then please feel free to create a topic on the google group (https://groups.google.com/forum/?fromgroups#!forum/locomotivecms), I think this sort of discussion in a more publicly viewable place would be beneficial.

[kikito]

Alright! Keep up the good work guys!