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

Adding support for DataMapper as a model #811

Closed
wants to merge 2 commits into from

Conversation

TrangPham
Copy link
Contributor

Connected to #83

Please note that I have currently refrained from implementing heavier SQL dependant functions such as .near, due to lack of knowledge on DataMapper.

@alexreisner
Copy link
Owner

Thanks for this. Unfortunately I don't think we're going to add support for DataMapper. I should have closed that old issue with a comment to that effect. The focus of Geocoder is on the process of geocoding. The near method (and other query methods) are provided as a convenience to support the most common use cases for lat/lon data. The Geocoder::Sql module is provided for anyone who wants to implement distance queries using another object-relational mapper (see the full_distance method for implementing near).

Sorry about this. I'm going to close #83.

@alexreisner
Copy link
Owner

@TrangPham Thanks so much for all of your recent work on Geocoder. It is very much appreciated! If you're interested in thinking about database-related problems, you might be interested in tackling one of the biggest and longest-standing problems in Geocoder: testing database queries. Currently ActiveRecord is stubbed and they are simply not tested. I am honestly not sure of the best way to test them and have not thought about this problem in a couple of years, but it really needs to be solved.

If you're interested, I think the first step is opening an issue where we can discuss and fully describe the problem. Ideally we want to run real queries on a real database, and do so in multiple versions of rails (3.0.x, 3.1.x, 3.2.x, 4.0.x, 4.1.x, 4.2.x) and against multiple database types (PostgreSQL, MySQL, SQLite, MongoDB). I've used the geocoder_test project for running these tests in the past but it's out of date and I don't think the approach is very good anyway. I'm not sure how other gems handle this kind of testing. Maybe what I'm proposing is overkill, but we've had enough Rails version- and DB type-specific bugs over the years that I think this level of detail is warranted.

Having a more complete test suite will also make me feel more comfortable adding DB-related features, as requested by many of the old PRs (eg, DataMapper support).

This is a big task and might be more than you want to take on. Obviously, feel free to contribute whatever you like.

@TrangPham
Copy link
Contributor Author

@alexreisner I'm interested, though I would need your input on the direction you would want it to go. Can you create the issue for it and we'll talk more on there about what specific steps need to be taken to actualize testing like that?

Thank you

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.

None yet

2 participants