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

Detect Swiftpm support #1412

Merged
merged 12 commits into from
Oct 29, 2024
Merged

Conversation

sigurdm
Copy link
Contributor

@sigurdm sigurdm commented Oct 24, 2024

Fixes #1402

@sigurdm sigurdm requested review from isoos and jonasfj October 24, 2024 12:50
@sigurdm
Copy link
Contributor Author

sigurdm commented Oct 24, 2024

cc @loic-sharma.

Copy link
Collaborator

@isoos isoos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with the variable rename

lib/src/tag/pana_tags.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@isoos isoos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: please update the CHANGELOG

@sigurdm sigurdm requested a review from szakarias October 25, 2024 08:14
Copy link
Member

@jonasfj jonasfj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, if @loic-sharma is happy.

I'm a little unsure if darwin/ should only be attempted when sharedDarwinSource: true is the case.

lib/src/report/multi_platform.dart Show resolved Hide resolved
lib/src/report/multi_platform.dart Outdated Show resolved Hide resolved
lib/src/report/multi_platform.dart Show resolved Hide resolved
Comment on lines 375 to 378
final specificPackageSwiftFile =
path.join(darwinOs, packageName, 'Package.swift');
final genericPackageSwiftFile =
path.join('darwin', packageName, 'Package.swift');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loic-sharma, the only documentation I can find for things under flutter.plugin is the guide guide. It doesn't even default to telling people to use Package.swift, it does reference swiftpm docs though.

I see that the docs suggest that you can use properties like:

  • ffiPlugin, and,
  • sharedDarwinSource.

Under flutter.plugin.platforms.ios and flutter.plugin.platforms.macos.

Does ffiPlugin only affect method channels, and not affect the need for a podspec or Package.swift? (I'm not sure entirely sure, just guessing)

Is sharedDarwinSource: true the only circumstance under which we should look in darwin/<packageName>/Package.swift ? (I'm suspecting so)


It would probably be useful with Flutter plugin reference documentation.
Similar to pubspec reference docs covering the properties under flutter.plugin and what files they imply exist.

Copy link

@loic-sharma loic-sharma Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does ffiPlugin only affect method channels, and not affect the need for a podspec or Package.swift? (I'm not sure entirely sure, just guessing)

From my understanding, a plugin can support CocoaPods & SwiftPM and also be an ffiPlugin. For example: https://docs.flutter.dev/platform-integration/ios/c-interop#open-source-third-party-library

cc @dcharkes to confirm.

Is sharedDarwinSource: true the only circumstance under which we should look in darwin/<packageName>/Package.swift ? (I'm suspecting so)

Yup that's correct.

If flutter.plugin.platforms.ios.sharedDarwinSource is true, we should check the darwin/<plugin name>/Package.swift. If this config is false or omitted, we should check the ios/<plugin name>/Package.swift.

Keep in mind that iOS and macOS have separate sharedDarwinSource configs. It's possible that only one platform uses the darwin directory. So if flutter.plugin.platforms.ios.sharedDarwinSource is true but flutter.plugin.platforms.macos.sharedDarwinSource is omitted, we should use the darwin directory for iOS but the macos directory for macOS.

It would probably be useful with Flutter plugin reference documentation.
Similar to pubspec reference docs covering the properties under flutter.plugin and what files they imply exist.

This is an excellent idea. I filed this issue: flutter/website#11334

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigurdm should we take sharedDarwinSource into account?

sigurdm and others added 2 commits October 25, 2024 13:27
Co-authored-by: Jonas Finnemann Jensen <[email protected]>
Co-authored-by: Jonas Finnemann Jensen <[email protected]>
@loic-sharma
Copy link

This is awesome, thanks for adding this! 💙

Copy link

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks again for adding this!

@sigurdm sigurdm merged commit 857cb1b into dart-lang:master Oct 29, 2024
5 checks 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.

Score support for swift package manager in plugins with ios/macos support
4 participants