martinrybak / SQLClient

Native Microsoft SQL Server client for iOS
http://objcsharp.wordpress.com/2013/10/15/an-open-source-sql-server-library-for-ios/
MIT License
124 stars 51 forks source link

Support for Swift Package Manager #71

Open imarennow opened 5 years ago

martinrybak commented 5 years ago

Thanks very much! Can you provide me with instructions on how to properly test this? Also do you have any suggestions on making the code more Swift-friendly? Thanks!

imarennow commented 4 years ago

Sure thing! A few notes, enumerated:

  1. I noticed after making the PR that my updates to the README file didn't jive with Github's Markdown parser, which I've corrected in a new commit.
  2. I've attached two zips, which contain a sample Xcode project (you'll need the latest version of Xcode for that) and a "plain" Swift Package Manager project respectively.
    2.1. The projects only demonstrate how to reference the repo via SwiftPM and that the code compiles, since I don't have a SQL Server to connect to for demo purposes.
    2.2. The SwiftPM references are to our branch, for obvious reasons, so those demo projects will break if/when we delete our repo due to a merge.
  3. As far as increased Swift-friendliness, you've already done the biggest piece, which is nullability annotations. The only place where I see some room for improvement is:
    3.1. The declaration of -execute:completion:, which leaves the contained type of the completion parameter unspecified. If it were specified as - (void)execute:(nonnull NSString*)sql completion:(nullable void(^)(NSArray<NSArray<NSDictionary<NSString*, __kindof NSObject*>*>*>* results))completion;, then it would be imported into Swift as func execute(_ sql: String, completion: @escaping (Array<Array<Dictionary<String, AnyObject>>>)->Void), which makes accessing the results a bit easier.

Sorry for the delay.

SwiftSQL_Xcode.zip

SwiftSQL_spm.zip

sree-86 commented 3 years ago

I have been using this since about a year and everything seems to be working fine. This pull request can be merged.