Skip to content

[geolocator_apple] Add swift package manager compatibility #1630

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

Conversation

alejandro-all-win-software
Copy link
Contributor

Makes geolocator_apple available as a Swift Package to Flutter

Closes #1629

Pre-launch Checklist

  • I made sure the project builds.
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is does not need version changes.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I rebased onto main.
  • I added new tests to check the change I am making, or this PR does not need tests.
  • I made sure all existing and new tests are passing.
  • I ran dart format . and committed any changes.
  • I ran flutter analyze and fixed any errors.

@alejandro-all-win-software alejandro-all-win-software changed the title Add swift package manager compatibility [geolocator_apple] Add swift package manager compatibility Mar 4, 2025
@alejandro-all-win-software
Copy link
Contributor Author

Hey @TimHoogstrate, any chance this PR could be reviewed soon? Thanks!

…into swift-package-manager-compatibility

# Conflicts:
#	geolocator_apple/CHANGELOG.md
#	geolocator_apple/macos/geolocator_apple.podspec
@TimHoogstrate
Copy link
Contributor

Hey @TimHoogstrate, any chance this PR could be reviewed soon? Thanks!

Thanks @alejandro-all-win-software for the contribution. I'm currently reviewing your work. It looks good to me, but what would be the best approach to test this?

Kind regards,

@alejandro-all-win-software
Copy link
Contributor Author

Hey @TimHoogstrate, if you’re referring to just testing the integration, you can enable SPM for Flutter using the following command:

flutter config --enable-swift-package-manager

This is mentioned in the Flutter documentation.

If you mean testing it in workflows, we could take a similar approach to flutter/packages and run a build with SPM enabled. We could also incorporate a tool like this:
flutter/packages build examples command.

@TimHoogstrate
Copy link
Contributor

Oke, i've tested this and it seems to work as expected. You also renamed iOS (and macOS) to Darwin, seems logical to me but @mvanbeusekom what do you think of it?

@mvanbeusekom
Copy link
Member

Google names the platform specific implementations after the SDK that is being used. Some examples:

  • path_provider and shared_preferences use the Foundation SDK internally so the platform package is called path_provider_foundation and shared_preferences_foundation
  • video_player uses the AVFoundation SDK and the platform package is therefore called video_player_avfoundation.
  • webview_flutter uses the WKWebKit SDK which is why the package is called webview_flutter_wkwebkit.

The reason they do this is that they can easily add more implementations when Apple introduces a new SDK. Working closely together with the Google team I like this convention. Which mean for the geolocator I would prefer the name geolocator_corelocation after the Core Location SDK that we utilize.

@alejandro-all-win-software
Copy link
Contributor Author

@mvanbeusekom, should I rename the package from geolocator_apple to geolocator_corelocation? Also, should I update class names that include 'Apple' accordingly?

@mvanbeusekom
Copy link
Member

Yes, if we are renaming that would be my preference.

That way we'll stay close to Google's take on naming packages for the Apple platforms.otherwise I don't really see a need to rename geolocator_apple to geolocator_darwin. Especially since Apple it self seems to have stepped away from using the name "Darwin".

@alejandro-all-win-software
Copy link
Contributor Author

alejandro-all-win-software commented Mar 20, 2025

Hmm, I didn't rename the entire package to geolocator_darwin, just the internal ios folder to darwin, following the convention used in other packages that combine both iOS and macOS logic. For example, the Video Player package also names that folder darwin.
Screenshot 2025-03-20 at 9 03 34 AM

@mvanbeusekom
Copy link
Member

Sorry for the confusion, I was working from my mobile phone and didn't complete get the whole picture/ context. In this case I am fine with renaming the folder to darwin as it is a better indication of the fact that the code is shared between the iOS and macOS platforms.

@TimHoogstrate, this looks good to me and can be merged.

@TimHoogstrate TimHoogstrate merged commit 48beaf0 into Baseflow:main Mar 21, 2025
1 check passed
TimHoogstrate pushed a commit that referenced this pull request Apr 1, 2025
* chore: added swift package manager compatibility

* chore: bump pubspec version

* chore: added ephemeral to gitignore
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.

[Feature Request]: Add Swift Package Manager support
3 participants