search

geocoder-php / Geocoder

The most featured Geocoder library written in PHP.
https://geocoder-php.org
MIT License
3.94k stars 517 forks source link

feat: support api key on here provider #1042

Closed atymic closed 3 years ago

atymic commented 4 years ago

I've updated the provider so it can use both the API key and appcode/appkey. The issue is that's a bit of a mess. Since the appcode/appkey is deprecated, maybe we should consider bumping the major version and dropping support for the old style auth?

Closes #1041

jbelien commented 4 years ago

Thanks @atymic ! 👍 I'll review it but could you make sure it passes the tests first.

AppCode doesn't seem to be completely deprecated, they are still usable but new users apparently can't create it anymore (see https://developer.here.com/documentation/authentication/dev_guide/topics/app-credentials.html) Therefore I would prefer to keep both options possible. What's your opinion @Nyholm ?

atymic commented 4 years ago

I'll review it but could you make sure it passes the tests first.

They actually do all pass locally, something is wonky with CI. Will try to figure it out

AppCode doesn't seem to be completely deprecated, they are still usable but new users apparently can't create it anymore (see developer.here.com/documentation/authentication/dev_guide/topics/app-credentials.html) Therefore I would prefer to keep both options possible.

While I did implement it with both options, on second thoughts I don't think there's much point it having both. Anyone still on the old auth method can switch over via the dashboard pretty easily, and nothing is going to break for existing users (due to the major version bump)

Anyone new coming to the package can use the next major version with the API key.

atymic commented 4 years ago

Ok, i've tried redoing the cache files with no luck. Any ideas why they are not working in CI?

image

alteredorange commented 4 years ago

So is Here not currently working? I just signed up, and can't get app code credentials, but when I just put in an API key, it says I need an app code...

pfuhrmann commented 3 years ago

Would be great to get this merged. The current state makes here-provider unusable for any new user or any user that needs to refresh the credentials.

@jbelien do you have ideas about why CI is failing? Can I help somehow to get this merged @atymic ?

jbelien commented 3 years ago

@jbelien do you have ideas about why CI is failing? Can I help somehow to get this merged @atymic ?

I'm having the same issue locally ... Investigating it now :)

jbelien commented 3 years ago

@atymic Could you add some documentation and merge the Style CI fix we did in master ? Thanks.

atymic commented 3 years ago

On it :)

atymic commented 3 years ago

@jbelien docs added and style issues resolved :)

jbelien commented 3 years ago

Awesome! Thanks @atymic 🎉

jbelien commented 3 years ago

Version 0.5.0 is released.