search

nerdishbynature / octokit.swift

A Swift API Client for GitHub and GitHub Enterprise
MIT License
490 stars 126 forks source link

Add Swift 5.1 support on Linux #85

Closed ellneal closed 4 years ago

ellneal commented 4 years ago

Uses the FoundationNetworking module where available.

ellneal commented 4 years ago

@f-meloni I actually did this to get my fork of danger building on linux with Swift 5.1 (and you're right that this is the last thing missing) 😂

I was going to wrap these in #if os(Linux) but thought it could get a bit ugly, so was waiting for feedback from the maintainers before making it even uglier 😄

f-meloni commented 4 years ago

I think #if os(Linux) would not work because swift doesn't have FoundationNetworking before swift 5.1

JosephDuffy commented 4 years ago

I have opened https://github.com/ellneal/octokit.swift/pull/1, which adds the #if swift(>=4.1) check.

Another option could be to drop Xcode 9.2?

Swift 4.1 (which is when canImport was introduced) requires Xcode 9.3, but it could also be argued that only Xcode 10 and Xcode 11 are to be supported (last 2 major versions).

pietbrauer commented 4 years ago

I would rather support iOS versions than Xcode versions. I think it’s ok to support only iOS 10, 11 and 12.

ellneal commented 4 years ago

Switching to Xcode 11 has just created a hang when installing gems. I'll look into it when I have some free time.

pietbrauer commented 4 years ago

Maybe you can take a hint from RequestKit as we are on 10.2 “already”. I also have an open pull request which is failing for a different reason using Xcode 11.2

https://github.com/nerdishbynature/RequestKit

ellneal commented 4 years ago

I have actually managed to get all the tests to pass now, but it’s timing out when attempting to lint the pod spec 🤦‍♂️

pietbrauer commented 4 years ago

Yes, so same as on RequestKit. Maybe disable it for now and open an issue so we can move on?

ellneal commented 4 years ago

@pietbrauer it looks to me as if this is all controlled from the nerdishbynature/requestkit_fastlane repository. My experience with Fastlane is entirely contained within the commits in this PR, so I'm not sure I'm the best person for the job 😅

ellneal commented 4 years ago

I've managed to disable the pod spec lint step by removing the DEPLOY_PODSPEC environment variable 🎉

ellneal commented 4 years ago

@pietbrauer No worries 😁 Can you tag a new release? I'll open an issue to re-enable the podspec linting 👍

f-meloni commented 4 years ago

Yeah a new release would be great 🚀

pietbrauer commented 4 years ago

@ellneal @f-meloni Done