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

Migrate to Swift 5 #2232

Merged
merged 9 commits into from
Oct 2, 2019
Merged

Migrate to Swift 5 #2232

merged 9 commits into from
Oct 2, 2019

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Sep 16, 2019

Migrated to Swift 5, for now using the Swift 5.0 migrator in Xcode 10. The idea is to have as clean a slate as possible for the Objective-C obsolescence work in #2231.

Recap:

  • Migrate to Swift 5.0
  • Migrate to Swift 5.1 – too soon to require Xcode 11
  • Fix switch exhaustiveness warnings

Blocked by #2206.

/cc @mapbox/navigation-ios

@1ec5 1ec5 added the op-ex Refactoring, Tech Debt or any other operational excellence work. label Sep 16, 2019
@1ec5 1ec5 added this to the v0.38.0 milestone Sep 16, 2019
@1ec5 1ec5 self-assigned this Sep 16, 2019
@1ec5
Copy link
Contributor Author

1ec5 commented Sep 16, 2019

The migration introduced a number of warnings:

/path/to/mapbox-navigation-ios/MapboxCoreNavigation/MBNavigator.swift:21:9: Switch covers known cases, but 'MBRouteState' may have additional unknown values
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/MBNavigator.swift:21:9: Handle unknown values using "@unknown default"
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/EventDetails.swift:354:9: Switch covers known cases, but 'UIApplication.State' may have additional unknown values, possibly added in future versions
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/EventDetails.swift:354:9: Handle unknown values using "@unknown default"
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/DistanceFormatter.swift:39:9: Switch covers known cases, but 'LengthFormatter.Unit' may have additional unknown values, possibly added in future versions
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/DistanceFormatter.swift:39:9: Handle unknown values using "@unknown default"
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/DistanceFormatter.swift:70:13: Switch covers known cases, but 'LengthFormatter.Unit' may have additional unknown values, possibly added in future versions
/path/to/mapbox-navigation-ios/MapboxCoreNavigation/DistanceFormatter.swift:70:13: Handle unknown values using "@unknown default"
/path/to/mapbox-navigation-ios/MapboxNavigation/StatusView.swift:118:13: Switch covers known cases, but 'UIUserInterfaceLayoutDirection' may have additional unknown values, possibly added in future versions
/path/to/mapbox-navigation-ios/MapboxNavigation/StatusView.swift:118:13: Handle unknown values using "@unknown default"
/path/to/mapbox-navigation-ios/MapboxNavigation/Style.swift:484:13: Switch covers known cases, but 'UIUserInterfaceLayoutDirection' may have additional unknown values, possibly added in future versions
/path/to/mapbox-navigation-ios/MapboxNavigation/Style.swift:484:13: Handle unknown values using "@unknown default"

Some of these issues can be addressed by removing LengthFormatter.Unit usage in favor of Measurement<UnitLength> as part of either #2206 or #2114. As for switch exhaustiveness warnings, we can accept a fatalError() on some of them, but others can probably be turned into simple if-else statements.

@1ec5
Copy link
Contributor Author

1ec5 commented Sep 16, 2019

Some of the builds are failing because Swift 5 isn’t supported in the Xcode 10.1 / iOS 9.3 toolchain. #2206 would remove this build in favor of an Xcode 11 / iOS 13 toolchain.

@1ec5 1ec5 mentioned this pull request Sep 18, 2019
@1ec5 1ec5 merged commit af602fe into master Oct 2, 2019
@1ec5 1ec5 deleted the 1ec5-swift5 branch October 2, 2019 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant