PromiseKit / CoreLocation

Promises for Swift & ObjC
http://promisekit.org
MIT License
30 stars 29 forks source link

Wrapped geocodePostalAddress into Promise #12

Closed timbms closed 6 years ago

timbms commented 6 years ago

I have wrapped geocodePostalAddress into a Promise and created a test for it.

mxcl commented 6 years ago

Thanks @timbms this is a very good PR.

CI says we have trouble on tvOS and Xcode 8.3: https://travis-ci.org/PromiseKit/CoreLocation/builds/345128586

tvOS has trouble importing Contacts, perhaps we can drop the import? The CNPostalAddress hopefully will get imported via the function you are using.

I'm not sure about Xcode 8.3, you specify the availability restrictions, so it should not be being compiled.


I also see that apple provides this function and:

func geocodePostalAddress(CNPostalAddress, preferredLocale: Locale?, completionHandler: CLGeocodeCompletionHandler)

Where if you provide nil to preferredLocale it acts the same as the above, so I think we should only use the second and provide nil as the default parameter. Thus we support both.


LMK if I can help.

timbms commented 6 years ago

Hi @mxcl I committed an updated version. Can you see it? I also added switches: exclude tvOS (function geocodePostalAddress is not contained there).

I opted not to go for the same function with default parameters. Just to be fully aligned with apple's version. Tim

mxcl commented 6 years ago

For future reference, you can just run the tests locally for all platforms. The Xcode 8.3 failures are harder to test without CI though. Personally I have a copy of Xcode 8.3.

Anyway, I've grabbed your PR locally and am fixing it.

timbms commented 6 years ago

Max can you please give me a kickstart on how to run the test locally for all platforms.

Also, it is not clear to me how I can install the necessary prerequisites. For instance I just wanted to go ahead with a PR for Alamofire PromiseKit with the injection of a JSONDecoder to handle different time formats. I tried now swift's package builder. This was also without success. After I was unsuccessful with Xcode manual completions of the dev environment.

Regards Tim

2018-02-24 17:49 GMT+01:00 Max Howell notifications@github.com:

For future reference, you can just run the tests locally for all platforms. The Xcode 8.3 failures are harder to test without CI though. Personally I have a copy of Xcode 8.3.

Anyway, I've grabbed your PR locally and am fixing it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PromiseKit/CoreLocation/pull/12#issuecomment-368241612, or mute the thread https://github.com/notifications/unsubscribe-auth/AFKRO6vPb3COgXZsZB-jlGkOMnVDd1WIks5tYD2DgaJpZM4SQdfg .

mxcl commented 6 years ago

can you please give me a kickstart on how to run the test locally for all platforms.

  1. Open Xcode
  2. Select desired platform
  3. Run tests

Also, it is not clear to me how I can install the necessary prerequisites.

Run carthage bootstrap.