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

Support for Swift Package Manager #71

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

imarennow
Copy link

No description provided.

@martinrybak
Copy link
Owner

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
Copy link
Author

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

sree-86 commented Apr 22, 2021

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

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.

None yet

3 participants