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

⚡️ Enhance locationiq - formattedAddress, county, unify country name #335

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

a-tokyo
Copy link

@a-tokyo a-tokyo commented Mar 10, 2022

⚡️ Enhance locationiq geocoder:

  • add formattedAddress
  • add county
  • unify country name

closes #336
closes #337

@nchaulet
Copy link
Owner

For the unify country name it is still an issue? Did you try to contact locationIQ so they can fix the problem on their side? I am not a fan of hardcoding data here, (if it's the only solution maybe but I would like us to try to see if the provider can fix their data before)

@a-tokyo
Copy link
Author

a-tokyo commented Apr 10, 2022

@nchaulet Hey! Thanks for the response ( :

I am not a fan of hardcoding either, however, unfortunately requesting locationIQ to change their API with a breaking change is not feasible.

If you think I should remove the country name unification part, let me know. I already hardcoded the fix in my backend so this wouldn't affect my needs.

I think the other changes in this PR are crucial tho!

@nchaulet
Copy link
Owner

Hi @a-tokyo Yes I think it probably make sense to remove the country specificity here, otherwise great job here, as soon you removed the hardcoded country it should be good to go

@a-tokyo
Copy link
Author

a-tokyo commented Apr 15, 2022

@nchaulet Done! 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants