wit-ai / wit-ios

Wit.ai iOS client
Other
222 stars 71 forks source link

Major Revision of iOS SDK #69

Closed hactar closed 8 years ago

hactar commented 8 years ago

So I've rewritten many parts of the SDK while maintaining API compatibility with the previous SDK version (4.1.0). It's what I use for https://subzero.eu/wave/

Features

There are a few changes I made that might not make everyone happy:

thediary commented 8 years ago

Let's please get this merged into the main branch of the SDK! There are so many great improvements in here, and after testing some of them in our app, I think they would be a welcome addition into the main library.

spencerp commented 8 years ago

Looking great! Thanks for making these changes! Most of the comments are nits. The only things I really care about are: 1) Making location sharing in the context default off, but configurable. 2) Double-checking the project settings to make sure all of the changes were intentional

hactar commented 8 years ago

@SpencerP: Thanks for the thorough code check, always glad when there's more pair of eyes on code :) regarding your comments 1) location sharing: While I agree that making it optional would be a nice solution, its not enough to simply make the calls optional, as the current code isn't very clean:

So until someone rewrites this code (I'll happily accept a PR to my PR:)) I humbly propose that not having it in is better than having this version of the code in.

2) I looked over the project settings again and to me they appear fine, but honestly I'm not an expert on all Xcode project settings. The changes come from:

spencerp commented 8 years ago

Sounds good! I'll see about refactoring to expose the location option later, but looks like that will be too big a change for this diff.