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

Add support for activesupport gem? #200

Open
mtho11 opened this issue Mar 17, 2017 · 12 comments
Open

Add support for activesupport gem? #200

mtho11 opened this issue Mar 17, 2017 · 12 comments

Comments

@mtho11
Copy link
Member

mtho11 commented Mar 17, 2017

I'm just reiterating @cfcosta question in irc about adding support for activesupportgem. It sounds like a good idea to me as it will make our code more rails idiomatic.

Thoughts?

@cfcosta
Copy link
Contributor

cfcosta commented Mar 17, 2017

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.

@josejulio
Copy link
Member

I also think we can benefit from having that dependency. 👍

@josejulio
Copy link
Member

Although we might want to wait for #196 before refactoring anything.

@abonas
Copy link
Contributor

abonas commented Mar 19, 2017

I missed the beginning of the discussion - please provide some examples as to why this is beneficial to this particular gem.
Please also see an issue that we encountered in another gem which actually caused us to drop the dependency in active support.
ManageIQ/kubeclient#182

@cfcosta
Copy link
Contributor

cfcosta commented Mar 19, 2017

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

@pilhuhn
Copy link
Member

pilhuhn commented Mar 20, 2017 via email

@pilhuhn
Copy link
Member

pilhuhn commented Mar 20, 2017

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.

@abonas
Copy link
Contributor

abonas commented Mar 20, 2017

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.

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.
And I also would like to make sure as @pilhuhn that this will continue working in a non Rails env, although I am pretty sure it should.

@cfcosta
Copy link
Contributor

cfcosta commented Mar 20, 2017

@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 railties gem). Also, we do not need to require everything, just the extensions we need, like requiring active_support/core_ext/blank if we need blank?. By doing both we avoid polluting the user namespace too much and still have the niceties for ourselves.

@josejulio
Copy link
Member

This is what we currently test in travis.
We stopped testing against 2.0 and 2.1 in #165.

@abonas
Copy link
Contributor

abonas commented Mar 23, 2017

so lets bump the gemspec, especially if we don't test 2.0 and 2.1 anyway.
(if we don't test it, how can we support it?)

@josejulio
Copy link
Member

Did we ever include this ?

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

No branches or pull requests

5 participants