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

feat: swift package manager #242

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

Conversation

EchoEllet
Copy link
Contributor

@EchoEllet EchoEllet commented Nov 23, 2024

Overview

Adds support for SPM following the Flutter instructions.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.

Related Issues

@EchoEllet EchoEllet requested a review from natsuk4ze as a code owner November 23, 2024 15:44
Copy link
Contributor Author

@EchoEllet EchoEllet left a comment

Choose a reason for hiding this comment

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

A quick review of the changes.

.gitignore Show resolved Hide resolved
darwin/gal.podspec Show resolved Hide resolved
darwin/gal/Package.swift Outdated Show resolved Hide resolved
example/macos/Podfile.lock Show resolved Hide resolved
example/macos/Runner/AppDelegate.swift Show resolved Hide resolved
example/macos/Runner/AppDelegate.swift Show resolved Hide resolved
@EchoEllet
Copy link
Contributor Author

nit: I have tested both CocoaPods and SPM within the example (turning on and off SPM) but it might be a good idea to automate this check in CI, We often don't need this since this one package and it would complicate the process.

@EchoEllet
Copy link
Contributor Author

Collection literals should not have trailing commas
.process("PrivacyInfo.xcprivacy"),

This doesn't apply to Swift code (false positive).

@natsuk4ze
Copy link
Owner

Thank you for your contribution @EchoEllet !

This doesn't apply to Swift code (false positive).

Can you follow the rules?

@EchoEllet
Copy link
Contributor Author

EchoEllet commented Nov 24, 2024

Can you follow the rules?

Single quotes (') are not used for string literals in Swift:

// This won't compile
.process('PrivacyInfo.xcprivacy'),

This warning is in the file Package.swift, it's not possible to use 'PrivacyInfo.xcprivacy' instead of "PrivacyInfo.xcprivacy".

@natsuk4ze
Copy link
Owner

I think the warning is for trailing comma @EchoEllet

@EchoEllet
Copy link
Contributor Author

Files should have a single trailing newline

I have added a single trailing newline to the file Package.swift though it still fails.

]
)
]
)
Copy link
Owner

Choose a reason for hiding this comment

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

we need the new line here
9e7544efd52bc2acdc7c9b36ba1c04e1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running wc -l darwin/gal/Package.swift outputs 25 when it's 26 lines in the code editor and actual file, it's likely a configuration issue in Git (related to line-endings) or the editor in my local machine. I added another line from my end so it appears only one empty line at the end from remote.

@natsuk4ze
Copy link
Owner

natsuk4ze commented Nov 24, 2024

@EchoEllet Oh no. Please do not stop adding Privacy Manifest. It is fine without it, but adding an empty Privacy Manifest is a better choice.

Can you revert 83f710d?

@EchoEllet
Copy link
Contributor Author

but adding an empty Privacy Manifest is a better choice.

By empty, I think they meant empty values, they said Only shared_preferences has a non-empty manifest, (see PrivacyInfo.xcprivacy of shared_preferences). All the other plugins including image_picker has the same template with default values.

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.

Add Swift Package Manager Support
2 participants