On 21/08/12 16:23 +0200, Jiří Stránský wrote:
On 21.8.2012 15:35, Jason Guiditta wrote:
As aeolus-image will no longer be used once the rails
engine/factory/v2 integration are done, I think keeping it isolated in
that gem makes the most sense. You will have to make a smaller
number of changes to the code when compared to catching every instance
where conductor uses it, so it just feels like a more effective
approach to me. If we were to keep this gem for some reason, I don't
see any reason the version check could not be removed in the future.
If we wanted to do that, we could just return a sensible error that
showed the proper api call, or better yet, just alias the method call
and allow either version to instantiate it (provided this doesn't
cause some security problem). However, I don't expect we will have to
do this, as all the ARes stuff is currently built into the new engine.
Even if we split it out, I think it will probably end up as a new gem
rather than an update to the current one.
Not sure if we understood each other here :)
I'm ok with making the change on the aeolus-image side, I just don't want to make it a
mixin. I want to change ...::Factory::Base#initialize directly. My "what do you
think" question was related to the issue of making a mixin vs. modifying directly.
I have no problem with that, mixin idea was more thinking it would be
used for both factory and iwhd, which I didnt recall does not use ARes
(yay for this going away soon...)
The version check can be definitely removed, but the code that will make
Conductor work when using ActiveResource 3.1 (and higher) can't be removed
unless you change the way Conductor calls aeolus-image methods. So that's why I
don't want to make it a mixin, the ActiveResource 3.1+ code will have to stay
there even if we remove the version check.
Well, Conductor _will_ change in it's interaction with ARes, as all
those call will be encapsulated in the new engine, so any other
conductor controllers will only be calling local models, which will in
turn make any factory updates that are needed. The net is, go for what
you described, and we will be ditching 3.0.x support in the next major
release anyway.
-j
Sorry for not being clearer in my last mail.
I'm gonna submit another patch and we'll see if we agree on it :)
J.
