search

bgentry / flipper-activerecord

DEPRECATED - A Flipper adapter for ActiveRecord
MIT License
4 stars 16 forks source link

Use flipper-activerecord with AR3.2 #2

Closed jesseproudman closed 9 years ago

jesseproudman commented 9 years ago

Added instructions on how to use flipper-activerecord with activerecord 3.2.21 using the https://github.com/matthuhiggins/foreigner gem.

Still working through the finer details here.

jesseproudman commented 9 years ago

Well, this isn't super pretty to make it compatible with Rails 3.2, but it is working. Would love any thoughts / comments?

bgentry commented 9 years ago

@blueboxjesse wasn't watching this repo, sorry I just saw this. I don't have time to look through the PR now but I'll get to it soon. Feel free to ping me if I haven't reviewed in the next couple days :)

jesseproudman commented 9 years ago

Hey, no worries. We're using my forked version now in production - Let me know if I can answer any questions.

bgentry commented 9 years ago

oh, crap, I see that most of these changes were made because the finder methods I used are new to Rails 4 and therefore you can't use them with Rails 3. That's unfortunate, as it seems to dirty up the code a fair bit :(

My inclination is to let you keep Rails 3.2 compatibility in your fork. I'd be willing to merge any changes necessary for 4.X support (removing the foreign key and lowering AR version requirements), but I'd rather not merge in changes that use more verbose or less clear ActiveRecord queries just for the sake of compatibility with a no-longer-supported version of Rails.

If you'd like, you can submit a PR to add a readme change that links to your fork for those wanting Rails 3.2 support.

Sorry this took me so long to review!

jesseproudman commented 9 years ago

No problem. Makes total sense. Opened a new PR here: https://github.com/bgentry/flipper-activerecord/pull/3