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.
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.
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.