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

[WIP] Swift 3.0 #94

Merged
merged 15 commits into from
Feb 6, 2017
Merged

[WIP] Swift 3.0 #94

merged 15 commits into from
Feb 6, 2017

Conversation

marcelofabri
Copy link
Contributor

@marcelofabri marcelofabri commented Sep 7, 2016

This adds support to Swift 3.0. It's still WIP:

let substring = string.substringWithRange(range)

escaped += substring.stringByAddingPercentEncodingWithAllowedCharacters(allowedCharacterSet) ?? substring
var allowedCharacterSet = CharacterSet.urlQueryAllowed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this from Alamofire code. It seems that they are dropping iOS 8.x on the new release. Maybe we should do the same? (If not, this needs to be reverted and converted to Swift 3.0)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view it is ok to drop iOS 8 support. If someone needs it, we are open for a Pull Request :)

@marcelofabri
Copy link
Contributor Author

Also, I chose to not mark the classes as open as the converter suggested, but that may be discussible

@marcelofabri
Copy link
Contributor Author

I think Circle hasn't update to the GM yet

github "jspahrsummers/xcconfigs" "0.9"
github "ReactiveCocoa/ReactiveCocoa" "v4.2.2"
github "ReactiveCocoa/ReactiveCocoa" "v4.0.0-alpha.3"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason, this was changed when I updated Nimble

¯_(ツ)_/¯

@marcelofabri
Copy link
Contributor Author

marcelofabri commented Sep 19, 2016

So, the macOS and iOS builds are passing, but the Reactive Cocoa isn't. From what I've seen, 4.x won't support Swift 3, so we'd need to wait for 5.0. I have no idea if master is currently stable or not as I have little familiarity with RC.

Thoughts?

@SusannProszak
Copy link
Contributor

hello @marcelofabri
thanks for your pr. 👍
we prefer to wait until ReactiveCocoa releases swift 3.0 support. Until then I would leave the pr in [WIP].

@tesths
Copy link

tesths commented Sep 24, 2016

So, is there any plan to migrate Heimdallr to Swift 3.0?
I already use Xcode 8😔Recently I want to update my project to support swift 3.0.

@marcelofabri
Copy link
Contributor Author

Should we use https://github.com/ReactiveCocoa/ReactiveSwift instead of RAC 4? If so, there's already a 1.0.0-alpha.1 release we can use.

@marcelofabri
Copy link
Contributor Author

I've updated the dependencies now that Nimble and Quick have stable versions and also updated the code to use ReactiveSwift instead of ReactiveCocoa.

However, I had to comment all the Objective-C compatibility section from Heimdallr+ReactiveCocoa because I didn't know how to migrate that.

@Kalzem
Copy link

Kalzem commented Oct 12, 2016

Excuse me, I don't have much details into the project but as a user of Heimdallr, I would like to understand why is there - historically - blocking dependencies still here?
There was already a problem with the JSON parser before (Argo).
And now RAC.

Or is it that we can DL this branch, exclude RAC, and get things working?

@marcelofabri
Copy link
Contributor Author

@BabyAzerty If you use CocoaPods, you can use my fork without the RAC subspec (it's optional), so you can migrate your project to Swift 3.0 while waiting this PR to be merged. Just change your Podfile:

pod 'Heimdallr', git: 'https://github.com/marcelofabri/Heimdallr.swift.git', branch: 'swift-3.0'

While I can't speak for the maintainers, I think it'd be useful to make the RAC extension another repository to make it easier to manage, but I don't know how complex that would make things.

@bckr
Copy link
Contributor

bckr commented Oct 26, 2016

I've updated the dependencies now that Nimble and Quick have stable versions and also updated the code to use ReactiveSwift instead of ReactiveCocoa.

@marcelofabri Thank you so much for this and sorry for our late response here. We've been a bit busy the last couple of weeks but I would like to rekindle the discussion here again.

However, I had to comment all the Objective-C compatibility section from Heimdallr+ReactiveCocoa because I didn't know how to migrate that.

I think if we want to have full compatibility with Objc and Swift we'll require both ReactiveCocoa and the ReactiveObjCBridge now because the repository will be split up into multiple smaller ones for the upcoming release that brings Swift 3 compatibility.

While I can't speak for the maintainers, I think it'd be useful to make the RAC extension another repository to make it easier to manage, but I don't know how complex that would make things.

I like that idea! I believe it would make the project way easier to maintain. Since we are also internally moving away from heavy RAC usage in our project, it might be the right direction for Heimdallr as well. The reactive extensions can still live in a separate repository. I'll discuss it internally and comment here if we have made a final decision. Otherwise the Swift 3 migration would be blocked until we get a stable RAC 5.0 release.

@bckr
Copy link
Contributor

bckr commented Nov 2, 2016

We decided that we are fine with splitting up the repository and removing RAC as a dependency for this repository.

@marcelofabri Do you want to tackle that? I think you already provide a working Swift 3 fork without the RAC subspec? We'll create a second repository for the reactive extensions when we have a stable RAC release. Let me know if I can help you out in any way 😊

@marcelofabri
Copy link
Contributor Author

@bckr I'm currently traveling, so it's unlikely that I'm able to work on this for the next ~2 weeks. The fork currently have a RAC subspec, but it's just not working 100%.

I've allowed edits from maintainers, so feel free to start from my fork if you wanna handle this.

@bckr
Copy link
Contributor

bckr commented Nov 2, 2016

@marcelofabri Will do! Thanks!

# Conflicts:
#	Heimdallr/Core/Heimdallr.swift
#	Heimdallr/Core/OAuthAccessTokenDefaultParser.swift
#	Heimdallr/Core/OAuthAccessTokenParser.swift
#	HeimdallrTests/Core/HeimdallrSpec.swift
@Ingibjorg
Copy link

Hi all,
what is the ETD on this update? Heimdallr is the only library that prevents me from being able to update my project from Swift 2.3 to Swift 3.0. Xcode 8.2 will be the last release that will support Swift 2.3 so we should migrate quickly to Swift 3.0.

@jPaolantonio
Copy link

Is there anything I can do to help the Swift 3 migration?

@ugoArangino ugoArangino merged commit 5fc552d into trivago:master Feb 6, 2017
@marcelofabri marcelofabri deleted the swift-3.0 branch September 3, 2019 07:19
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.

10 participants