search

pelias / wof-admin-lookup

Who's on First Admin Lookup for the Pelias Geocoder
https://pelias.io
MIT License
9 stars 24 forks source link

Merge this repo back into pelias/admin-lookup #48

Open orangejulius opened 8 years ago

orangejulius commented 8 years ago

It feels rather silly to have two admin lookup repos, especially when one of them is deprecated. I propose we move all the code in this one into the pelias/admin-lookup repo, and then delete this one. We may change how we do admin lookup again in the future, so locking Who's on First into the title of the repository doesn't feel right.

trescube commented 8 years ago

Do it, I introduced the wof-admin-lookup project in my younger more inexperienced JS days.

dianashk commented 8 years ago

I kinda like having the data source be explicitly called out in the name. It would be ideal if there was also a generic admin-lookup module above this that served as a factory and users could specify which type of admin data they wanted to use. That would allow us or someone in the community to write an osm-admin-lookup for example and then use pelias-config to swap it out without having any impact on the rest of the codebase.

orangejulius commented 8 years ago

That's true, I like that as well. We do kinda have that with https://github.com/pelias/wof-pip-service/ though.

dianashk commented 8 years ago

Just to clarify, because I think my previous statement may have been a bit vague... An importer would do something like this:

var admin_lookup = require('pelias-admin-lookup');
var config = { admin_lookup: { source: 'wof' } };

someStreams
  .pipe(admin_lookup.setup(config.admin_lookup)
  .pipe(.....);

and in the admin-lookup code it would just create the right one based on the config.

orangejulius commented 8 years ago

Sorry, my comment was also vague! :p

I really like the interface you suggested and was also thinking of something along those lines when reading your comment, and was in fact agreeing. Right now we have wof-pip-service, which has a lot of, but not all of, the logic for doing admin lookup with WOF. Maybe that could eventually become wof-admin-lookup, and this repo could be admin-lookup.

In fact this also looks like it would help solve #52, since right now a little bit too much setup is required by code calling this module.