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

Workout route locations #20

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

Conversation

aplekhanov
Copy link
Contributor

Added a new method to get workout's route locations.

@mahnuh
Copy link
Collaborator

mahnuh commented Apr 11, 2023

Hi @aplekhanov,
thanks for your contribution! Sorry for the delayed review, we were pretty busy the last couple of weeks.
Two things:

  1. Have not been able to test it yet, but does importing CoreLocation trigger any kind of location access approval dialog? Like " wants to access your current location."
  2. I think the plugin is getting little to big now for just a single file with the actual method and all the helpers. It could be the right time to separate functionality into different files. Would you mind starting by moving your new stuff to a dedicated file or what ever makes sense?

@aplekhanov
Copy link
Contributor Author

Hi @mahnuh 👋
Thank you for taking the time to review my pull request and for your helpful suggestions. Please don't worry about the delay, I completely understand how busy things can get. I'll do my best to use your ideas in my code and make the necessary changes.

@mahnuh
Copy link
Collaborator

mahnuh commented Apr 11, 2023

@aplekhanov sounds great! I will review the changes again as soon as possible. A release with your other PR is on the way and will be landing today I guess. :-)

@kyllerss
Copy link

I'm glad @aplekhanov submitted this PR. Any ideas when it will be merged?

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.

3 participants