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

[NT-1499] Xcode 12/iOS 14.0 Compatibility #1303

Merged
merged 5 commits into from
Sep 24, 2020
Merged

[NT-1499] Xcode 12/iOS 14.0 Compatibility #1303

merged 5 commits into from
Sep 24, 2020

Conversation

justinswart
Copy link
Contributor

@justinswart justinswart commented Sep 23, 2020

📲 What

Updates our codebase for compatibility with Xcode 12.0 and builds the app against the iOS 14.0 SDK.

🤔 Why

Always good to keep things up to date, Xcode 12.0, Swift 5.3 and iOS 14.0 all bring improvements that we would like to have access to in our workflow.

🛠 How

  • Addressed some broken code as as result of changes in Swift 5.3, in particular we were affected by the Swift Evolution proposal regarding forward-scan trailing closures.
    • The proposal: SE-0286.
    • Related issue opened on ReactiveSwift.
    • Bug reported on Swift Forum: SR-13453.
  • Implemented a Carthage workaround until a fix is available, described here. This is required for our dependencies to build currently.
  • Re-recorded all snapshots. Unfortunately this is usually necessary when Xcode and iOS versions change due to tiny pixel changes in rendering.

✅ Acceptance criteria

  • App passes full regression testing.

# Conflicts:
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_de_device_pad@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_de_device_phone4_7inch@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_de_device_phone5_8inch@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_es_device_pad@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_es_device_phone4_7inch@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_es_device_phone5_8inch@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_ja_device_pad@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_ja_device_phone4_7inch@2x.png
#	Screenshots/_64/Kickstarter_Framework_iOSTests.RewardsCollectionViewControllerTests/testRewards_Backer_LiveProject_Landscape_lang_ja_device_phone5_8inch@2x.png
@codecov
Copy link

codecov bot commented Sep 24, 2020

Codecov Report

Merging #1303 into master will increase coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1303   +/-   ##
=======================================
  Coverage   85.68%   85.69%           
=======================================
  Files        1108     1108           
  Lines       96766    96780   +14     
=======================================
+ Hits        82915    82935   +20     
+ Misses      13851    13845    -6     
Impacted Files Coverage Δ
Library/ViewModels/RootViewModel.swift 0.00% <0.00%> (ø)
...arter-iOS/Views/DiscoveryProjectCategoryView.swift 100.00% <100.00%> (ø)
Library/ViewModels/ActivitiesViewModel.swift 98.26% <100.00%> (ø)
Library/ViewModels/ChangeEmailViewModel.swift 100.00% <100.00%> (ø)
Library/ViewModels/CommentsViewModel.swift 100.00% <100.00%> (ø)
...y/ViewModels/DashboardReferrersCellViewModel.swift 98.01% <100.00%> (ø)
Library/ViewModels/DiscoveryFiltersViewModel.swift 97.35% <100.00%> (ø)
Library/ViewModels/DiscoveryViewModel.swift 98.19% <100.00%> (ø)
Library/ViewModels/ManagePledgeViewModel.swift 100.00% <100.00%> (ø)
Library/ViewModels/PledgeViewModel.swift 97.79% <100.00%> (+0.01%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd75f3c...8c175d1. Read the comment docs.

@justinswart justinswart marked this pull request as ready for review September 24, 2020 01:17
.map { $0 && $1 }
.map { isEmailVerified, isEmailDeliverable -> Bool in
let r = isEmailVerified && isEmailDeliverable
return r
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are kind've unfortunate and seem unnecessary but I wasn't able to get this code to compile without introducing some of these local scope vars.

@@ -95,7 +95,7 @@ public final class CommentsViewModel: CommentsViewModelType, CommentsViewModelIn
self.viewDidLoadProperty.signal,
self.userSessionStartedProperty.signal
)
.map { AppEnvironment.current.currentUser }
.map { _ in AppEnvironment.current.currentUser }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works around the Swift function overload bug.

generateStandardViewControllers()
}
let personalizedViewControllers = userState.map { userState -> [RootViewControllerData] in
generatePersonalizedViewControllers(userState: (userState.isMember, userState.isLoggedIn))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was necessary to work around something that was deprecated in Swift called tuple shuffling: https://noahgilmore.com/blog/tuple-shuffling/

@justinswart justinswart changed the title [NT-1498] Xcode 12/iOS 14.0 Compatibility [NT-1499] Xcode 12/iOS 14.0 Compatibility Sep 24, 2020
Copy link
Contributor

@singhhari singhhari left a comment

Choose a reason for hiding this comment

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

lgtm!

@justinswart justinswart merged commit 1249d15 into master Sep 24, 2020
@justinswart justinswart deleted the xcode-12 branch September 24, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants