-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add support for activesupport gem? #200
Comments
I'm 👍 to that. It simplifies a lot some parts of code, and the tradeoff (having another maybe heavy dependency) is not that big imho. |
I also think we can benefit from having that dependency. 👍 |
Although we might want to wait for #196 before refactoring anything. |
I missed the beginning of the discussion - please provide some examples as to why this is beneficial to this particular gem. |
@abonas what's our target version of Ruby (and Rails)? Enforcing 2.2.2+ seems good to me, given that it's almost 2 years old. |
On 20 Mar 2017, at 0:30, Cainã Costa wrote:
@abonas what's our target version of Ruby (and Rails)? Enforcing
2.2.2+ seems good to me, given that it's almost 2 years old.
I think that is fine.
|
While it looks like this would work also in non-Rails environments, I want to make sure it indeed does. I would hate to lose the ability to use our client gem outside of Rails. |
Right now the gemspec says >2.0 . I don't have any objection to bump it, but in the current situation 'as is' adding activesupport will get us into an issue. |
@pilhuhn @abonas it works just fine, activesupport is not rails-bound at all (there are some stuff used by rails, but they are only bound in the |
so lets bump the gemspec, especially if we don't test 2.0 and 2.1 anyway. |
Did we ever include this ? |
I'm just reiterating @cfcosta question in irc about adding support for
activesupport
gem. It sounds like a good idea to me as it will make our code more rails idiomatic.Thoughts?
The text was updated successfully, but these errors were encountered: