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 Melissa Data to API Options #1521

Merged
merged 4 commits into from
Sep 1, 2021
Merged

Add Melissa Data to API Options #1521

merged 4 commits into from
Sep 1, 2021

Conversation

ALacker
Copy link
Contributor

@ALacker ALacker commented Aug 25, 2021

No description provided.

Copy link
Owner

@alexreisner alexreisner left a comment

Choose a reason for hiding this comment

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

This is great! Thanks very much. It's really close to merge-able, I've just made a few comments about minor issues. Let me know if you have any questions.

end

def test_raises_api_key_exception
Geocoder.configure Geocoder.configure(:always_raise => [Geocoder::InvalidApiKey])
Copy link
Owner

Choose a reason for hiding this comment

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

This looks like a possible typo (two Geocoder.configures).

Comment on lines 42 to 48
def latitude
@data['Latitude'].to_f
end

def longitude
@data['Longitude'].to_f
end
Copy link
Owner

Choose a reason for hiding this comment

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

The convention for Geocoder lookups is to implement a coordinates method that returns [lat, lon] rather than latitude and longitude methods (which are implemented in the base lookup).

@@ -165,6 +165,17 @@ The [Google Places Search API](https://developers.google.com/maps/documentation/
* **Limitations**: ?
* **Notes**: You can use the open (non-licensed) API by setting: `Geocoder.configure(mapquest: {open: true})` (defaults to licensed version)

### Melissa Data (`:melissa`)
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like Melissa also offers IP address geocoding, so maybe we want to call this lookup :melissa_street, in case we want to add :melissa_ip later?

@alexreisner alexreisner merged commit 50a0e6b into alexreisner:master Sep 1, 2021
@alexreisner
Copy link
Owner

Perfect! Thanks very much.

@ALacker ALacker deleted the melissa branch September 1, 2021 17:51
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.

2 participants