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

Opt out from extending Active Record #851

Closed
morgoth opened this issue Apr 29, 2015 · 5 comments
Closed

Opt out from extending Active Record #851

morgoth opened this issue Apr 29, 2015 · 5 comments
Labels

Comments

@morgoth
Copy link

morgoth commented Apr 29, 2015

Currently it's impossible to not load AR extensions in Rails apps.
Would you consider a change to extend AR just by including explicit some geocoder module?
Something like:

class User < ActiveRecord::Base
  include Geocoder::ActiveRecord
end

If no, maybe option in configuration to opt out from it would be possible to add?

@alexreisner
Copy link
Owner

Yes, that seems reasonable. I would consider a pull request.

@morgoth
Copy link
Author

morgoth commented Apr 29, 2015

Which version do you prefer?
The first one looks more clear, but it would be backward incompatible.
I believe this also apply to mongoid and mongomapper extensions

@alexreisner
Copy link
Owner

Definitely don't want to introduce anything that would break anyone's app. Maybe configuration is the way to go.

@morgoth
Copy link
Author

morgoth commented Apr 29, 2015

I tried with adding config, but looks like it's impossible. Geocoder.configure is usually placed in initializer, where the rails application is already loaded, thus it's after the gem configuration done in Railtie, where we would like to opt out based on user set config.

I'm also not sure what would be the best way of deprecating it, if we decide to switch to explicit extend:

class User < ActiveRecord::Base
  extend Geocoder::Model::ActiveRecord
end

@alexreisner
Copy link
Owner

While I acknowledge the benefits of this, I don't like the idea of requiring an explicit extend. First priority is not to break apps (which includes deprecating things). But like I said, this is desirable functionality and I remain open to a pull request.

Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants