Skip to content

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
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