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

Add methods and classes for Places (New) API in flutter_google_places_sdk_platform_interface #87

Merged
merged 7 commits into from
Aug 27, 2024

Conversation

flikkr
Copy link
Contributor

@flikkr flikkr commented Jun 16, 2024

Description

Add classes and methods introduced in Google Places (New) API. I have some changes to add to Android library once this is merged. Tracked by #77.

@matanshukry
Copy link
Owner

hey @flikkr , thanks for the PR!
There was a discussion not too long ago: #65
The reason I'm not so sure about moving to the new api is that least comment.

If you can comment on that with some sources from the documentation that would be great - otherwise I would want to do more research before deciding to move.

Regardless:

  1. I've added a couple of review, take a look when you can
  2. If you can also create a PR on how you use it - that would be great. That 2nd pr doesn't have to be perfect nor even build, but it's hard to comment on this without knowing how it will even be used.

@flikkr
Copy link
Contributor Author

flikkr commented Jul 24, 2024

Hi @matanshukry, thanks for taking the time to look into this! I've created a sample PR #91 to show how I integrate the full feature in my project.

Regarding the concern on moving to the new API, there should not be any breaking changes as the new API can only be used once it is enabled via the Google Cloud console and initialised in app by calling Places.initializeWithNewPlacesApiEnabled. The old API can still be used by calling Places.initialize. (see migration guide)

@matanshukry
Copy link
Owner

@flikkr again sorry for the long delays;
Anyway let's just create a separate enum for the new api 🙏 , and we can merge this after

@matanshukry
Copy link
Owner

matanshukry commented Aug 17, 2024

@flikkr I fixed the tests and workflow in master, after resolving conflicts it's now missing the return null statement.
Let's fix it and make sure the tests passes, and merge this

@flikkr
Copy link
Contributor Author

flikkr commented Aug 26, 2024

Hi @matanshukry I fixed the failing test, please help to run the workflow

@matanshukry matanshukry merged commit 67dfb02 into matanshukry:master Aug 27, 2024
1 check passed
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.

2 participants