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: prep for Linux, Windows, and Android support #1

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

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Feb 5, 2022

Hey @reddavis, just saw your framework earlier today on Swift Package Index and I think it's promising, great work!

I think this can be very useful for those who use ParseSwift.

Please have a look at this PR for preparing the project for adding Linux, Windows, and Android support. There are some methods used related to DispatchSource that won't allow for building on Linux, Windows, and Android. In the future if these become supported or if there are other methods used, this framework can work on those OS's.

Let me know what you think...

Updates in this PR:

  • Only build Combineand Logger functionality on Apple Platforms.
  • Bump Swift tools to 5.5. This will allow you to add DocC support directly along with other things
  • Add tvOS and watchOS to Package.swift, the tests pass locally for these. You might want to add builds for these in a future PR
  • Add Linux and Windows to the CI (commented out for now). This shows what prevents the build. Let me know if you want me to remove these
  • Add Codecov
  • Fix a warning in Package.swift

@cbaker6 cbaker6 changed the title feat: add Linux, Windows, and Android support feat: prep for Linux, Windows, and Android support Feb 5, 2022
@@ -55,7 +64,11 @@ public final class PapyrusStore {
do {
try self.createDirectoryIfNeeded(at: self.url)
} catch {
#if !os(Linux) && !os(Android) && !os(Windows)
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder whether it is cleaner to bring these checks inside the Logger class. So the Logger class decides whether to print or use os log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remember correctly, Linux, Android, and Windows don't have os.log, so it can't go directly in the logger. I'm not sure if those OS's have an equivalent logger

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 20, 2022

Not sure what's wrong with the SPM build as it builds locally, but I can't get it working in the CI, so I commented it out.

As for Linux and Windows, you can see code needs to be updated to get build/test working here. I commented out Linux and Windows in the CI just incase you want to build test in the future.

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.

2 participants