Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Rails 4 #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

straydogstudio
Copy link

Hey there. I've been updating axlsx_rails to support Rails 4.1, and found you need this change for acts_as_axlsx. The find(:all) method is no longer available.

I don't see any explicit documentation for passing options to the find(:all) method. I've guessed for Rails 4.1 that where(conditions) and order(order) would be useful. Perhaps the data option should be documented. Thoughts?

@andreorvalho
Copy link

  • 1 for this. can somebody merge this?

@straydogstudio
Copy link
Author

@andreorvalho In the meantime you can use the fork:

gem 'acts_as_axlsx', git: 'git://github.com/straydogstudio/acts_as_xlsx.git'

@randym seems busy elsewhere. Not sure when or if he will respond.

@phuong3030
Copy link

@straydogstudio: Thank you for fixing this issue.

@randym
Copy link
Owner

randym commented Jun 13, 2014

sorry - I will try to get to this on Sunday.

Again, apologies!! from a bad maintainer!

On Wed, Apr 30, 2014 at 11:15 PM, Noel Peden notifications@github.com
wrote:

@andreorvalho https://github.com/andreorvalho In the meantime you can
use the fork:

gem 'acts_as_axlsx', git: 'git://github.com/straydogstudio/acts_as_xlsx.git'

@randym https://github.com/randym seems busy elsewhere. Not sure when
or if he will respond.


Reply to this email directly or view it on GitHub
#10 (comment).

@straydogstudio
Copy link
Author

Nice to hear from you @randym! No worries.

@ricardodovalle
Copy link

👍 it will be great.
I have used @straydogstudio / @andreorvalho fork.

@ricardodovalle
Copy link

Only fixing the link:

gem 'acts_as_xlsx', git: 'git://github.com/straydogstudio/acts_as_xlsx.git'

@randym
Copy link
Owner

randym commented Jun 16, 2014

What we need here is support for ActiveRecord > 2.3.8, not a rails version. The problem with adding in a Rails::Version comparison is that it bakes in the assumption that we are running on Rails. Highly likely, but certainly not a requirement now that all that stuff has been cleanly packed into Gems.

I'll re-write this with tests today and add in a PR. I'd appreciate any feedback.

@straydogstudio
Copy link
Author

@randym Certainly. Didn't think of limiting it to ActiveRecord. I'll look for the PR.

@randym
Copy link
Owner

randym commented Jun 16, 2014

playing with https://github.com/thoughtbot/appraisal - looks like a respectable way to handle some of the different version scenarios in testing.

Any objections to doing a 2.0 and dropping support for 1.8.7? The ROI on maintaining compatibility is near nil at this point.

@straydogstudio
Copy link
Author

No objections from me re: dropping 1.8.7. People can always use an older version.

@randym
Copy link
Owner

randym commented Jun 17, 2014

Here be the PR
#11
I'll probably clean up the version switching a bit so it only has to happen in one place, but this is the basics.

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