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

Gs7 updateUpdate Here Geocode and Reverse Geocode Provider to GS7 (v7) #1231

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

SCatrinh
Copy link

@SCatrinh SCatrinh commented Jul 3, 2024

Updates Here provider to support GS7 under the 0.8.0 version tag. Details in linked issue

Closes #1230

SCatrinh added 18 commits June 24, 2024 17:12
@patrickghidossi
Copy link

patrickghidossi commented Jan 15, 2025

Synced with master @jbelien , thanks again for reviewing

@jbelien
Copy link
Member

jbelien commented Feb 16, 2025

Hello @patrickghidossi ,

Thanks for your contribution.

I had a first (quick) look to the changes made.
This looks more like a breaking change (and so a new major version) than just an update (and minor version).

Looking at some documentation, I've found this:

GS7 introduces a new way to authenticate the application. Instead of using the Appid/Appcode combination, you must use the API Key or OAuth 2.0 token authentication methods for improved security. App code credentials are now on the verge of deprecation as they are the least secure of the authentication credential types.
I still see some reference to AppId in your code, is it stil supported?

I see that you changed some tests, the goal of those tests is to make sure we don't introduce breaking changes ; please keep the tests as they are, only adapt what needs to be adapted. If there are new features in v7, feel free to add extra tests.

Before I'll review the code, could you already do the following:

  • Prepare version 1.0.0 instead of 0.8.0
  • Rebase to our master branch
  • Make sure all PHPStan checks are passing
  • Keep tests as they are

Thanks!

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

Successfully merging this pull request may close these issues.

Update Here Geocode and Reverse Geocode Provider to GS7 (v7)
3 participants