-
-
Notifications
You must be signed in to change notification settings - Fork 980
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
feat(battery_plus): Add Swift Package Manager support #3154
Conversation
59ed5b4
to
9ba32e4
Compare
9ba32e4
to
3e3ea9e
Compare
I would be happy to include flutter/website PR and feel free to review it. There is no need to rush and we would like to discuss how it should be implemented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @koji-1009 ! thanks for contribution. I ran pod lint on macos projects and get few warnings:
➜ battery_plus pod lib lint macos/battery_plus.podspec --configuration=Debug --skip-tests --use-modular-headers
-> battery_plus (0.0.1)
- WARN | license: Missing license type.
- WARN | [OSX] keys: Missing primary key for `source` attribute. The acceptable ones are: `git, hg, http, svn`.
- NOTE | xcodebuild: note: Using codesigning identity override:
- NOTE | [OSX] xcodebuild: note: Building targets in dependency order
- NOTE | [OSX] xcodebuild: note: Target dependency graph (4 targets)
- NOTE | [OSX] xcodebuild: note: Metadata extraction skipped. No AppIntents.framework dependency found. (in target 'battery_plus' from project 'Pods')
- NOTE | [OSX] xcodebuild: note: Metadata extraction skipped. No AppIntents.framework dependency found. (in target 'App' from project 'App')
[!] battery_plus did not pass validation, due to 2 warnings (but you can use `--allow-warnings` to ignore them).
You can use the `--no-clean` option to inspect any issue.
➜ battery_plus pod lib lint macos/battery_plus.podspec --configuration=Debug --skip-tests --use-modular-headers --use-libraries
-> battery_plus (0.0.1)
- WARN | license: Missing license type.
- WARN | [OSX] keys: Missing primary key for `source` attribute. The acceptable ones are: `git, hg, http, svn`.
- NOTE | xcodebuild: note: Using codesigning identity override:
- NOTE | [OSX] xcodebuild: note: Building targets in dependency order
- NOTE | [OSX] xcodebuild: note: Target dependency graph (4 targets)
- NOTE | [OSX] xcodebuild: note: Metadata extraction skipped. No AppIntents.framework dependency found. (in target 'App' from project 'App')
[!] battery_plus did not pass validation, due to 2 warnings (but you can use `--allow-warnings` to ignore them).
You can use the `--no-clean` option to inspect any issue.
not sure if they were there before but it would be nice to fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I think that warning were there before, so let's leave it as it is. Probably fix for different PR.
lgtm
The contents of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I still want to test this on my devices before merge.
resources: [] | ||
) | ||
] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: add new line here
s.dependency 'Flutter' | ||
|
||
s.platform = :ios, '12.0' | ||
s.pod_target_xcconfig = { 'DEFINES_MODULE' => 'YES' } | ||
s.resource_bundles = {'batery_plus_privacy' => ['PrivacyInfo.xcprivacy']} | ||
s.resource_bundles = {'batery_plus_privacy' => ['battery_plus/Sources/battery_plus/PrivacyInfo.xcprivacy']} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should batery
be battery
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some testing on iOS and MacOS and can confirm that everything works as it should in both SPM enabled and disabled modes.
Thanks a lot for your contribution. We are good to merge this PR!
Description
Add SPM support.
https://docs.flutter.dev/packages-and-plugins/swift-package-manager/for-plugin-authors
I also noticed an error in the documentation.
Since we need to refer to the documentation when reviewing PRs, it will be draft until the next PR is reviewed.
flutter/website#11020
As a working test, I have confirmed that it can be built with flutter 3.24 + CocoaPods, and that it can be built with flutter main channel + SPM.
Note that if you try building with main channel + SPM, you will need to remove some steps from your iOS project, as the script to add Pods is no longer needed.
Related Issues
Checklist
CHANGELOG.md
nor the plugin version inpubspec.yaml
files.flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!
in the title as explained in Conventional Commits).