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

Currently Incompatible with Geocoder #3

Open
RustComet opened this issue Feb 15, 2016 · 4 comments · May be fixed by #4
Open

Currently Incompatible with Geocoder #3

RustComet opened this issue Feb 15, 2016 · 4 comments · May be fixed by #4

Comments

@RustComet
Copy link
Contributor

Currently geo_query includes the following method:

# Returns all objects within a rectangle
Class.within_bounding_box(min_lat, min_lon, max_lat, max_lon)

However this clashes when also using the Geocoder gem as it contains the same method.

There is currently a branch that fixes this, but will break current implementations.

I think we should release and tag the current master as 0.1.0 (current Rubygems is 0.0.4) and then release a new version 0.2.0 after merging the PR.

Happy to write up a new Readme for the branch as well to make sure it clearly explains the difference between the releases.

@RustComet RustComet linked a pull request Feb 15, 2016 that will close this issue
@dawilster
Copy link
Contributor

@RustComet I think I've encountered this before.
I'll need to dig up where I used it but I remember instead of requiring the entire geo_coder gem I just included the individual modules I needed.

@RustComet
Copy link
Contributor Author

@dawilster That would work. But I think it's a simpler approach to have separate method names. I know it's not often that you would need both the geo_query and geocoder bounding box commands, but I think things like active_admin_tokeninput uses geocoder's bounding box doesn't it? So if you want to use that as well then you would run into issues

@dawilster
Copy link
Contributor

I don't think active_admin_tokeninput does, it just queries collections a name.

It just seems odd to me to update a gem when it conflicts with another gem. I think it would be good practice to think about what you would do if a gem you didn't control had a method name conflict.

Could look into monkey patching the gem by using alias_method http://apidock.com/ruby/Module/alias_method and just changing the method name.

@RustComet
Copy link
Contributor Author

That's a good point.

I think another way to approach this would be to write a replacement for geocoder altogether. At the moment geocoder is better for setting coordinates from the backend (geocoding from an address, which is really only used by admins, and not often in most implementations), and geo_query is better for querying.

Maybe we should write our own little geocoder style gem, that just does the setting. It would be more lightweight and better suited to working alongside geo_query, rather than installing geocoder and then only requiring parts we need.

Just writing as I think here, might not be worth it… but would be handy to control the whole thing

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 a pull request may close this issue.

2 participants