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

Remove Objective-C compatibility; adopt Swift language features #2230

Merged
merged 57 commits into from
Nov 20, 2019

Conversation

JThramer
Copy link
Contributor

@JThramer JThramer commented Sep 16, 2019

Obsolete. With extreme prejudice.

No man is an island,
Entire of itself.
Each is a piece of the continent,
A part of the main.
If a clod be washed away by the sea,
Europe is the less.
As well as if a promontory were.
As well as if a manor of thine own
Or of thine friend's were.
Each man's death diminishes me,
For I am involved in mankind.
Therefore, send not to know
For whom the bell tolls,
It tolls for thee.
-- For Whom The Bell Tolls, by John Donne

Work in progress.

Running Checklist:

  • Eliminate Objective-C support
    • Delete @objc attributes and MB class prefixes
    • Fix resultant build errors.
    • Delete methods whose sole purpose is to bridge to Objective-C
    • Fix broken Guidance Cards
    • Fix any other test failures
    • Move enumeration declarations from Objective-C headers to Swift files
    • Delete CMapboxDirections target from SPM distribution of MapboxDirections.swift
    • Remove references to Objective-C compatibility from readmes and jazzy guides
  • Take advantage of Swift language features
    • Convert value classes to structures or enumerations with associated types
    • Convert abstract classes to protocols with default implementations
    • Move global variables and global constants under structures and classes
  • Replace magic default values with optionals

Fixes #2231.

@JThramer JThramer added op-ex Refactoring, Tech Debt or any other operational excellence work. pair labels Sep 16, 2019
@JThramer JThramer added this to the v1.0.0 milestone Sep 16, 2019
@JThramer JThramer self-assigned this Sep 16, 2019
@JThramer JThramer changed the title 1.0 Objective-C Obsoletion [WIP] 1.0 Objective-C Obsoletion Sep 16, 2019
@JThramer JThramer force-pushed the jerrad/objc-delenda-est branch from 571ebde to 596fe6f Compare September 24, 2019 20:14
Example/ViewController.swift Outdated Show resolved Hide resolved
@JThramer
Copy link
Contributor Author

JThramer commented Sep 25, 2019

Some lessons learned so far:

There are a few unanticipated complications that may impact our refactoring objectives. Namely:

  • KVO is only possible through NSObject Inheritance (OBJC Dynamic Dispatch), and that affects classes like RouteProgress and NavigationSettings.
  • UIAppearance inherits from NSObjectProtocol, which pins Style.swift and DayStyle.swift to our current implementation without refactoring our theming mechanism.

@JThramer JThramer marked this pull request as ready for review September 27, 2019 00:52
@JThramer JThramer changed the title [WIP] 1.0 Objective-C Obsoletion 1.0 Objective-C Obsoletion Sep 27, 2019
MapboxCoreNavigationTests/DistanceFormatterTests.swift Outdated Show resolved Hide resolved
XCTAssertTrue(dependencies.navigationService.eventsManager.usesDefaultUserInterface, "MapboxCoreNavigationTests should have an implicit dependency on MapboxNavigation due to running inside the Example application target.")
}
// func testDefaultUserInterfaceUsage() {
// XCTAssertTrue(dependencies.navigationService.eventsManager.usesDefaultUserInterface, "MapboxCoreNavigationTests should have an implicit dependency on MapboxNavigation due to running inside the Example application target.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This test should be restored and fixed.

@1ec5
Copy link
Contributor

1ec5 commented Nov 13, 2019

I got a crash arriving at the final destination:

#0	0x00007fff518292c6 in __pthread_kill ()
#1	0x00007fff518d0bf1 in pthread_kill ()
#2	0x00007fff517b9a3c in abort ()
#3	0x00000001080e09e5 in swift::fatalError(unsigned int, char const*, ...) ()
#4	0x00000001080d8de7 in swift::swift_dynamicCastFailure(void const*, char const*, void const*, char const*, char const*) ()
#5	0x00000001080d8e5a in swift::swift_dynamicCastFailure(swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*, char const*) ()
#6	0x00000001080dafd3 in swift_dynamicCastClassUnconditional ()
#7	0x0000000105e780c3 in closure #1 in RouteMapViewController.endOfRouteViewController.getter at /path/to/mapbox-navigation-ios/MapboxNavigation/RouteMapViewController.swift:20
#8	0x0000000105e77f07 in RouteMapViewController.endOfRouteViewController.getter at /path/to/mapbox-navigation-ios/MapboxNavigation/RouteMapViewController.swift:18
#9	0x0000000105e85928 in RouteMapViewController.embedEndOfRoute() at /path/to/mapbox-navigation-ios/MapboxNavigation/RouteMapViewController.swift:363
#10	0x0000000105e86497 in RouteMapViewController.showEndOfRoute(duration:completion:) at /path/to/mapbox-navigation-ios/MapboxNavigation/RouteMapViewController.swift:384
#11	0x0000000105e31c41 in NavigationViewController.showEndOfRouteFeedback(duration:completionHandler:) at /path/to/mapbox-navigation-ios/MapboxNavigation/NavigationViewController.swift:539
#12	0x0000000105e319f9 in NavigationViewController.navigationService(_:didArriveAt:) at /path/to/mapbox-navigation-ios/MapboxNavigation/NavigationViewController.swift:532
#13	0x0000000105e32ca9 in protocol witness for NavigationServiceDelegate.navigationService(_:didArriveAt:) in conformance NavigationViewController ()
#14	0x0000000107c75cfd in MapboxNavigationService.router(_:didArriveAt:) at /path/to/mapbox-navigation-ios/MapboxCoreNavigation/NavigationService.swift:464
#15	0x0000000107c76529 in protocol witness for RouterDelegate.router(_:didArriveAt:) in conformance MapboxNavigationService ()
#16	0x0000000107c63425 in RouteController.updateRouteLegProgress(status:) at /path/to/mapbox-navigation-ios/MapboxCoreNavigation/RouteController.swift:275

The error is:

2019-11-12 19:05:27.940108-0800 Example-CarPlay[43862:4033688] [Storyboard] Unknown class MBEndOfRouteViewController in Interface Builder file.

Please audit the storyboards for any Objective-C class names.

@1ec5 1ec5 self-requested a review November 13, 2019 03:08
@1ec5 1ec5 dismissed their stale review November 13, 2019 03:08

Serious crashes due to stale references in storyboards.

MapboxCoreNavigation/UnimplementedLogging.swift Outdated Show resolved Hide resolved
MapboxNavigation/Error.swift Outdated Show resolved Hide resolved
@JThramer JThramer merged commit 9cbbd2f into master Nov 20, 2019
@JThramer JThramer deleted the jerrad/objc-delenda-est branch November 20, 2019 21:42
@1ec5 1ec5 changed the title 1.0 Objective-C Obsoletion Remove Objective-C compatibility; adopt Swift language features Nov 21, 2019
@1ec5 1ec5 mentioned this pull request Jan 6, 2020
10 tasks
@1ec5 1ec5 mentioned this pull request Mar 9, 2020
@1ec5 1ec5 modified the milestones: v1.0.0, v0.x next Apr 14, 2020
@1ec5 1ec5 mentioned this pull request Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible changes that break backwards compatibility of public API build Issues related to builds and dependency management. op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discontinue Objective-C compatibility
2 participants