Swift 3/Xcode 8 conversion #111

long · 2016-09-20T07:58:57Z

Ran the conversion to Swift 3.0 against a current project using the UberRides Cocoapod. Starting point was the swift-3-dev branch.

Updated ObjectMapper spec to ~> 2.0 to have the Swift 3 version of the dependency.

Objects with type Mappable were updated to to require the named parameter and not '_' in order to match specifications in the current version of ObjectMapper

After porting the source code, the project in this repo and test files were converted in Xcode to Swift 3.0 but not built/tested

long · 2016-09-20T08:02:10Z

Project using Cocoapod builds cleanly and is able to access Uber app with the specified destination (functionality used in project app)

jbrophy17 · 2016-09-21T02:24:04Z

Thanks for the PR! I'll go ahead and take a look at this, but before I can merge anything, could you sign the Uber CLA?

kwstasna · 2016-09-23T10:24:54Z

I got your branch and it worked for a simple app of mine. Just to open UBER with 2 locations etc. Nice work!!!

nbgraham · 2016-09-23T18:27:39Z

@long Thanks! This saved me a lot of headache trying to convert it myself. Now it builds successfully and runs correctly.

I had some trouble figuring out how to use @long 's version of the code in my XCode project. Here's what I did:

Clone @long 's fork of uber/rides-ios-sdk
git clone https://github.com/long/rides-ios-sdk.git
Switch to the swift-3-dev branch
cd rides-ios-sdk
git branch swift-3-dev
Switch to your Xcode project directory.
Set up Cocoapods.
pod init
Add UberRides to your Podfile, pointing to your local version (from Step 1; Mine was in ~/Developer/rides-ios-sdk)
pod 'UberRides', :path => '~/Developer/rides-ios-sdk'
Install and update your pods.
pod install
pod update
Rebuild your project!

kwstasna · 2016-09-23T18:37:21Z

@nbgraham Or you can do
pod 'UberRides', :git => 'https://github.com/long/rides-ios-sdk.git', :branch => 'swift-3-dev’

Inside your podfile and you will avoid 6 steps! 💃

nbgraham · 2016-09-23T18:48:17Z

@kwstasna Thanks, that's a lot better.

I got a lot of build errors the first time, until I used XCode 8 migration tool Edit -> Convert -> To Current Swift Syntax…

Then it said no update necessary, and I rebuilt to get it working.

Why doesn't it work straight away?

kwstasna · 2016-09-23T18:52:14Z

@nbgraham I really don't know. I can't reproduce because as soon as I installed the pod as I told you above, I got no errors from Uber Rides!

jbrophy17 · 2016-09-28T18:11:00Z

@long Again thanks for this PR, but I need you to sign the Uber CLA before I can merge this in

jbrophy17

Most of my comments are just nits, but the big issue is that the tests don't compile.
Seems like a lot of the Enum renaming didn't get reflected in the test cases

jbrophy17 · 2016-10-03T18:42:16Z

source/UberRides/BaseDeeplink.swift

+    fileprivate var waitingOnSystemPromptResponse = false
+    fileprivate var checkingSystemPromptResponse = false
+    fileprivate var promptTimer: Timer?
+    fileprivate var completionWrapper: ((NSError?) -> ()) = { _ in }


I think all of these can stay as private

jbrophy17 · 2016-10-03T19:02:23Z

source/UberRides/BaseDeeplink.swift

        }

        completionWrapper(error)
    }

    //Mark: App Lifecycle Notifications

-    @objc private func appWillResignActiveHandler(notification: NSNotification) {
+    @objc fileprivate func appWillResignActiveHandler(_ notification: Notification) {


This and the functions below can be regular private as well

jbrophy17 · 2016-10-03T19:05:00Z

source/UberRides/Configuration.swift

+    fileprivate static var defaultAccessTokenIdentifier: String?
+    fileprivate static var region : Region = .default
+    fileprivate static var isSandbox : Bool = false
+    fileprivate static var useFallback: Bool = true


all of these fileprivate can be private

jbrophy17 · 2016-10-03T19:07:47Z

source/UberRides/Configuration.swift

@@ -417,15 +417,15 @@ import WebKit

    // MARK: Private

-    private static func getPlistDictionary() -> [String : AnyObject]? {
-        guard let path = bundle.pathForResource(plistName, ofType: "plist"),
+    fileprivate static func getPlistDictionary() -> [String : AnyObject]? {


This and those below can be private

jbrophy17 · 2016-10-03T19:19:19Z

source/UberRides/EndpointsManager.swift

            var loginQuery = baseLoginQuery(clientID, redirect: redirect, scopes: scopes)
            let additionalQueryItems = queryBuilder(("response_type", "token"))

-            loginQuery.appendContentsOf(additionalQueryItems)
+            loginQuery.append(contentsOf: additionalQueryItems)


above you changed this to += additionalQueryItems, can we do that here too?

jbrophy17 · 2016-10-03T20:29:56Z

source/UberRides/RideRequestButton.swift

-    private func getImage(name: String) -> UIImage {
-        let bundle = NSBundle(forClass: RideRequestButton.self)
-        let image = UIImage(named: name, inBundle: bundle, compatibleWithTraitCollection: nil)
+    fileprivate func getImage(_ name: String) -> UIImage {


can be private

jbrophy17 · 2016-10-03T20:30:09Z

source/UberRides/RideRequestButton.swift

@@ -481,7 +481,7 @@ struct ButtonMetadata {
    var dropoffLatitude: Double?
    var dropoffLongitude: Double?
    var timeEstimate: TimeEstimate?
-    private var priceEstimateList: [PriceEstimate]?
+    fileprivate var priceEstimateList: [PriceEstimate]?


priceEstimateList

jbrophy17 · 2016-10-03T20:32:19Z

source/UberRides/RideRequestView.swift

        }
    }

    // MARK: Private

-    private func initialSetup() {
+    fileprivate func initialSetup() {


can be private

jbrophy17 · 2016-10-03T20:32:34Z

source/UberRides/RideRequestView.swift


        setupWebView()
    }

-    private func setupWebView() {
+    fileprivate func setupWebView() {


can be private

jbrophy17 · 2016-10-03T20:37:56Z

source/UberRides/RideRequestViewController.swift

+    fileprivate var accessTokenWasUnauthorizedOnPreviousAttempt = false
+    fileprivate var accessTokenIdentifier: String
+    fileprivate var keychainAccessGroup: String
+    fileprivate var loginCompletion: ((_ accessToken: AccessToken?, _ error: NSError?) -> Void)?


loginCompletion can be private

jbrophy17 · 2016-10-03T20:43:34Z

Also, @long you need you to sign the Uber CLA before I can merge this

long · 2016-10-04T08:28:08Z

Sorry, I hadn't logged into github directly for a bit and hadn't received email notifications.

I've just signed the Uber CLA

jbrophy17 · 2016-10-25T08:37:49Z

Hey @long, have you had a chance to look at my comments on your PR? Would love to get this merged!

vinayjn · 2017-03-28T17:39:40Z

@jbrophy17 when are you planning to release the Swift 3 SDK, with the release of Xcode 8.3, I cannot build the SDK with Swift 2.3.

vinayjn · 2017-06-14T07:56:51Z

Really sad to see inactive development for this SDK. Swift 4 is released and the SDK is still not updated for Swift 3.

arogal · 2017-06-14T16:19:38Z

Hi Vinay, Thank you for your patience. We are indeed picking back up on this in a few weeks. We'll be posting a plan for Swift 3/4 so everyone can keep informed. Thanks.

…

On Wed, Jun 14, 2017, 12:57 AM Vinay Jain ***@***.***> wrote: Really sad to see inactive development for this SDK. Swift 4 is released and the SDK is still not updated for Swift 3. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#111 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFExoGxjRiBUKCfO7WJ9nmfjahiVygydks5sD5JogaJpZM4KBVD5> .

edjiang · 2017-07-06T22:12:03Z

Closing since it looks like the bulk of the migration was taken care of in 892a779

Sorry this took so long, and thank you @long for all of your help here :)

cc @vinayjn since you were asking for updates :)

See #167 for installing the Swift 3 package via your package manager.

long added 3 commits September 15, 2016 15:49

Update podspec for swift 3 dependencies

0403d6d

Swift 3 conversion clean build against target project

9904056

Swift 3 conversion for UberRides project

7ba4b33

long changed the title ~~Swift 3 dev~~ Swift 3/Xcode 8 conversion Sep 20, 2016

nbgraham mentioned this pull request Sep 23, 2016

Swift 3 Branch Working Solution. #115

Closed

jbrophy17 suggested changes Oct 3, 2016

View reviewed changes

jbrophy17 mentioned this pull request Oct 25, 2016

Add Swift 3 / Xcode 8 support #60

Closed

wenxin3262 approved these changes Nov 3, 2016

View reviewed changes

jbrophy17 mentioned this pull request Dec 29, 2016

Convert SDK to Swift 3 Syntax #149

Closed

scottrhoyt mentioned this pull request Jan 6, 2017

Swift 3 + Carthage temporary fix #154

Closed

edjiang closed this Jul 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swift 3/Xcode 8 conversion #111

Swift 3/Xcode 8 conversion #111

long commented Sep 20, 2016

long commented Sep 20, 2016

jbrophy17 commented Sep 21, 2016

kwstasna commented Sep 23, 2016

nbgraham commented Sep 23, 2016

kwstasna commented Sep 23, 2016

nbgraham commented Sep 23, 2016

kwstasna commented Sep 23, 2016

jbrophy17 commented Sep 28, 2016

jbrophy17 left a comment

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016 •

edited

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 Oct 3, 2016

jbrophy17 commented Oct 3, 2016

long commented Oct 4, 2016

jbrophy17 commented Oct 25, 2016

vinayjn commented Mar 28, 2017

vinayjn commented Jun 14, 2017

arogal commented Jun 14, 2017 via email

edjiang commented Jul 6, 2017

Swift 3/Xcode 8 conversion #111

Swift 3/Xcode 8 conversion #111

Conversation

long commented Sep 20, 2016

long commented Sep 20, 2016

jbrophy17 commented Sep 21, 2016

kwstasna commented Sep 23, 2016

nbgraham commented Sep 23, 2016

kwstasna commented Sep 23, 2016

nbgraham commented Sep 23, 2016

kwstasna commented Sep 23, 2016

jbrophy17 commented Sep 28, 2016

jbrophy17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrophy17 Oct 3, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrophy17 commented Oct 3, 2016

long commented Oct 4, 2016

jbrophy17 commented Oct 25, 2016

vinayjn commented Mar 28, 2017

vinayjn commented Jun 14, 2017

arogal commented Jun 14, 2017 via email

edjiang commented Jul 6, 2017

jbrophy17 Oct 3, 2016 •

edited