search

HeroTransitions / Hero

Elegant transition library for iOS & tvOS
https://HeroTransitions.github.io/Hero/
MIT License
22.03k stars 1.72k forks source link

Swift 5 Migration #645

Closed nick-potts closed 3 years ago

nick-potts commented 4 years ago

I redid the migration as per #594 , it also resolves #611 . Additionally, it added quite a few redundant roundtrip helper functions so I removed them.

nick-potts commented 4 years ago

CI is down :/

JoeMatt commented 4 years ago

Yes, GitHub changed their default macOS image for their docker environment. Mac is actually outsourced to Orka, and for some reason a couple of weeks ago they changed what their default SDK installs would be.

for open source projects, it's not a huge deal, you have to now install any but the default iOS simulator versions for each XCode version, which takes a minute or two. I hit this snag in my commercial repos where the time hit kind of sucks now that we gotta make a value cost for the extra minutes for every push.

I'll try to push a fix soon, but this PR would then have to merge or rebase that change.

https://www.macstadium.com/orka

image

Orka by MacStadium - Orchestration w/Kubernetes on Apple
Powered by MacStadium, Orka (Orchestration with Kubernetes on Apple) is a new virtualization layer for Mac build infrastructure based on Docker and Kubernetes technology. Orka offers the first and only solution for orchestrating macOS in a cloud environment using Kubernetes on genuine Apple hardware.
JoeMatt commented 4 years ago

Since your own pod is a pod for the demo app than no. Though you would inhibit all and then use false for just hero. Same difference

Joe Mattiello

From: Zonily Jame notifications@github.com Sent: Thursday, November 28, 2019 7:17:58 AM To: HeroTransitions/Hero Hero@noreply.github.com Cc: Joseph Mattiello mail@joemattiello.com; Review requested review_requested@noreply.github.com Subject: Re: [HeroTransitions/Hero] Swift 5 Migration (#645)

@kuyazee requested changes on this pull request.

In Podfilehttps://github.com/HeroTransitions/Hero/pull/645#discussion_r351749271:

@@ -3,7 +3,7 @@ target 'HeroExamples' do platform :ios, '9.0' use_frameworks!

  • pod 'CollectionKit'
  • pod 'CollectionKit', :inhibit_warnings => true

Shouldn't we inhibit warnings for all pods?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/HeroTransitions/Hero/pull/645?email_source=notifications&email_token=AADBT6BC5QMG366QC2S262LQV6ZHNA5CNFSM4JQ2AYW2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNJ2LJQ#pullrequestreview-324248998, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AADBT6GPFXEUSCC4KLZ3SGTQV6ZHNANCNFSM4JQ2AYWQ.

kuyazee commented 4 years ago

CI fixed in #648 let's rerun the CI after it gets merged

nick-potts commented 4 years ago

I rebased to get CI to run but its broken again haha

JoeMatt commented 4 years ago

Taking a look into the CI issue.

-- Something with the SwiftUI PR is breaking tests that should be skipped on iOS 10, and are working fine on my machine for iOS 13.2 🤷‍♂ Keep hacking at it on and off between my day job work.

WIP PR https://github.com/HeroTransitions/Hero/pull/651

JoeMatt commented 3 years ago

This was rebased, conflict resolved and merged in pr #696