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

Make MapBox interface compatible with MultiGeocoder #156

Merged
merged 1 commit into from
Feb 19, 2015
Merged

Make MapBox interface compatible with MultiGeocoder #156

merged 1 commit into from
Feb 19, 2015

Conversation

yitznewton
Copy link
Contributor

We're using MapBox with the MultiGeocoder, and Geokit's MapBox geocoder did not take the second options argument used by multi, resulting in errors like

Caught an error during Mapbox geocoding call: wrong number of arguments (2 for 1)

This PR fixes that by adding the argument to the appropriate methods, and ignoring it.

The tests simply confirm that no error is raised when geocoding with MapBox.

@@ -9,7 +9,7 @@ class MapboxGeocoder < Geocoder
private

# Template method which does the reverse-geocode lookup.
def self.do_reverse_geocode(latlng)
def self.do_reverse_geocode(latlng, options = {})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused method argument - options. If it's necessary, use _ or _options as an argument name to indicate that it won't be used.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) to 95.45% when pulling 7421a5a on yitznewton:fix-geocoder-duck-type into d3832a9 on geokit:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.16%) to 95.45% when pulling 7421a5a on yitznewton:fix-geocoder-duck-type into d3832a9 on geokit:master.

@yitznewton
Copy link
Contributor Author

Maintainers: let me know if I should act on any of this auto-feedback 😁

mnoack pushed a commit that referenced this pull request Feb 19, 2015
Make MapBox interface compatible with MultiGeocoder
@mnoack mnoack merged commit 00e2867 into geokit:master Feb 19, 2015
@mnoack
Copy link
Member

mnoack commented Feb 19, 2015

Thanks @yitznewton The auto feedback is usually fairly good to act on (apart from line-too-long is sometimes overzealous). This PR seems fairly good, safe and improves coverage so I'll just merge it.

I'm not sure when I should release a gem.

@yitznewton
Copy link
Contributor Author

👍

@yitznewton yitznewton deleted the fix-geocoder-duck-type branch February 19, 2015 23:36
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.

4 participants