On Dec 2, 2007 4:06 PM, Sven Fuchs <[EMAIL PROTECTED]> wrote: > Hi James, hi all,
Hi Sven, > Like we agreed on #engines last week I've worked on Engines for Rails 2.0 > (still using RC1) this weekend. I've commited my work here: > > http://svn.artweb-design.de/engines/tests/rails_2_0 I've checked out your code, and I'll be looking over it this week :) > As for the tests I had to invest some time to figure out how Al Pacino, > Baldwin, Stephen and William work together with James and how they all > cope with funk, bananas and hobbits! (Which was quite fun.) > > So I've decided to restructure this using some more descriptive names ... > hoping to increase comprehensibility a little. I hope this is ok for you. That seems fair :) > * ActionMailer breaks with the Engines::RailsExtension (tests disabled) > * $LOAD_PATH seems to be in reverse order so tests fail here (tests > disabled) > * no tests for arbitrary code mixing ported (unclear what's going to > drop) I believe the rendering mechanism for ActionMailer has changed, I'll investigate this. See Pascal's comments about the $LOAD_PATH - I believe it is in the correct order. I'm not sure that your test for this makes sense, but I'll double check this. While code mixing might be deprecated, it could be worth keeping parts of it around - for instance, I'm quite keen on moving the explicit declaration of "controllers and helpers" out of the depths of the Dependencies extensions to somewhere more visible. > As for the refactoring, I've changed quite a bit (mostly but not solely > responding to your considerations in IRC): > > * moved Rails.plugins to Engines.plugins This could be problematic, as existing migrations will refer to Rails.plugins. I suggest that we add a method to the Rails module which returns the result of a call to Engines.plugins. > * removed about.yml and version attribute > * removed Engines.current > * removed block from Engines::PluginList, left the rest there for now > * removed LegacySupport > * added an Engines::Assets module to hold file/dir creation/copying > related stuff > * added the following line in Engines::RailsExtensions::Routing > map = self # allows to just copy and paste routes around These all seem fine to me. > * moved extensions to an accessible attribute on Engines and > extensions loding from init.rb to Engines.init to declutter the init.rb file. > Engines and Engines::Plugin are pretty lean now. I moved stuff out of > init.rb because I thought it would be a nice idea to have the Engines plugin > behave just like a normal Rails plugin if it hasn't been "booted". I think > this > also responds to your consideration to mess as little with Rails core code as > possible. I'm not sure if I follow you here - by 'not booted', presumably you mean that the line hasn't been added to the top of environment.rb? > (On the other hand) I've added a default_attr_accessor method to Class > in order to dry up the repetition in Engine::Plugin. Personally I feel > that this extension to Class is quite useful (it could even go into > ActiveSupport). If you don't like it, I'll revert this. I sympathise, but I think it's possibly wise to avoid messing around with Ruby core classes unless it's unavoidable. I'd be more comfortable setting the defaults in a more explicit way. > Regarding the location of the routes.rb file: > ... why isn't the directory structure mirrored for the routes file? > I.e. why is it located in the plugin root dir instead of a /config subdir? > IMHO > mirroring the directory structure here would make things even more > intuitive. The distinction here is that the only file in our "config" directory would be the routes.rb file. Is it really worth creating a directory to hold that single file? Personally, I don't think so. I also feel that it's worth breaking away from the "your plugin is a mini application" mindset that mirroring the default app layout so completely would promote. Anyway - I need to spend some time merging what you've done into my own refactorings, and then I'm sure I'll have more to add. Thanks for your work so far Sven! Let me know if you have any other ideas, -- * J * ~ _______________________________________________ Engine-Developers mailing list [email protected] http://lists.rails-engines.org/listinfo.cgi/engine-developers-rails-engines.org
