search

stephencelis / SQLite.swift

A type-safe, Swift-language layer over SQLite3.
MIT License
9.57k stars 1.55k forks source link

Align platform versions in SPM manifest and Xcode project #1094

Closed calvincestari closed 2 years ago

calvincestari commented 2 years ago

This PR:

  1. Adds platform versions to the SPM manifest; we discovered this was missing while debugging https://github.com/apollographql/apollo-ios/pull/2015. This addition should allow SPM to correctly resolve deployment targets.
  2. Moves the Xcode deployment versions configuration from each target to the project level. There is some personal preference to this change but I do also believe there is value in being able to set all deployment versions in one place (project) vs. the individual targets where it's easy to miss versions not being the same. I also changed the tvOS target to match Cocoapods, which specifies 9.1.

I saw that https://github.com/stephencelis/SQLite.swift/pull/1083 already attempted this but that PR seems dead and the fork has also been deleted. If this one is merged you can close #1083.

Question:

  1. Is there any particular reason why SQLite.swift 0.13.0 bumped the macOS deployment target to 10.15? It seems at odds with the iOS deployment target of 9.0 which would have greater support for older APIs.
jberkel commented 2 years ago

Thanks for the PR, tests are failing. Regarding MacOS 10.15, I'm not sure, I see it was changed in #1051. Perhaps it can be reverted.

calvincestari commented 2 years ago

Thanks for merging that @jberkel!

@NathanFallet it looks like you made the change to the macOS version in the 0.13.0 release. I've built the project using the 'SQLite Mac' scheme with macOS deployment target 10.15 down back to 10.10 and I don't get any warnings; do you recall any specifics of that particular change?

jberkel commented 2 years ago

Seems to work fine on the CI with 10.10. If nobody objects, we can just set it back before making the next release. The problem is of course testing this on actual 10.10 systems.

nathanfallet commented 2 years ago

@calvincestari You're right, I changed the deployment target instead of the base SDK. Sorry for that mistake.

jberkel commented 2 years ago

@NathanFallet ok, I reverted it. It's probably worth doing a new release with the current master soon, can you take charge of it?

calvincestari commented 2 years ago

Hey @NathanFallet, no need to apologize. Thanks for the change both of you, I'm looking forward to getting our users onto the latest version.