Skip to content

Conversation

@bhcarpenter
Copy link

Implemented the suggestions from @RyanNaughton in (unrelated) #95.

Brandon Carpenter added 2 commits May 8, 2013 16:16
Rails 2.3 doesn't have Engine support, so disable that and let people
add the helpers themselves via an initializer.

Based on @RyanNaughton's comments here:
splitrb#95
@buddhamagnet
Copy link
Contributor

@andrew are we supporting Rails 2?

@RyanNaughton
Copy link

It is extremely easy to make it work with 2.3. We have apps ranging from rails 2.3 to rails 4 at Taxi Magic, and we'd like to be able to do experimentation across all of them equivalently. We have been using the same modifications that @bhcarpenter mentioned above, and it has been working fine.

@iangreenleaf
Copy link
Contributor

Yeah, given how unintrusive these changes are, I think it's a good idea. I pity those who are still on 2, but they definitely exist.

@phoet
Copy link
Contributor

phoet commented May 8, 2013

i think that it's the "successfull" user base, that's still on rails 2 with AB tests...

@bhcarpenter
Copy link
Author

At Scoutmob, we're actually planning to run an A/B test across three projects (2 x Rails 3, 1 x Rails 2.3) using a shared Redis.

We want to migrate that Rails 2.3 project, but something else always seems to be more pressing. By now we may just wait for Rails 4...

@andrew
Copy link
Member

andrew commented May 9, 2013

This looks like a simple enough change, looks fine to merge at the moment.

buddhamagnet added a commit that referenced this pull request May 9, 2013
@buddhamagnet buddhamagnet merged commit 84ed29c into splitrb:master May 9, 2013
@buddhamagnet
Copy link
Contributor

Merging...thanks for the changes @bhcarpenter!

@andrew
Copy link
Member

andrew commented May 9, 2013

Might as well update the travis config to test rails 2.3 as well

@bhcarpenter bhcarpenter deleted the rails2.3-compatibility branch May 10, 2013 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants