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 #382

Merged
merged 89 commits into from
Dec 11, 2019

Conversation

JThramer
Copy link
Contributor

@JThramer JThramer commented Sep 27, 2019

This is the MapboxDirections.swift-specific companion PR to mapbox/mapbox-navigation-ios#2230.

To-do:

Fixes #134, fixes #228, and fixes #381.

@JThramer JThramer added improvement Improvement for an existing feature. objective-c labels Sep 27, 2019
@JThramer JThramer added this to the 1.0 milestone Sep 27, 2019
@JThramer JThramer requested a review from 1ec5 September 27, 2019 00:50
@JThramer JThramer self-assigned this Sep 27, 2019
@1ec5
Copy link
Contributor

1ec5 commented Sep 27, 2019

This fixes #381.

This was referenced Sep 27, 2019
@1ec5
Copy link
Contributor

1ec5 commented Oct 2, 2019

…and fixes #134.

let response = options.response(from: json)
if let routes = response.1 {
self.postprocess(routes, fetchStartDate: fetchStartDate, responseEndDate: responseEndDate, uuid: json["uuid"] as? String)
guard let response = possibleResponse, ["application/json", "text/html"].contains(response.mimeType) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this method accept HTML-formatted responses while the other calculate methods only accept JSON?

codecov.yml Outdated
@@ -1,6 +1,6 @@
ignore:
- "Directions Example"
- "Tests"
- "MapboxDirectionsTests"
Copy link
Contributor

Choose a reason for hiding this comment

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

#360 originally intended to omit MapboxDirectionsTests from code coverage metrics, but when #362 renamed this folder to Tests, test code inflated test coverage by around 11%. We’ll need to adjust reapply this change in a separate commit outside of this PR to avoid getting dinged by the (justifiable) omission of test code from code coverage metrics.

@1ec5 1ec5 merged commit f2841b6 into master Dec 11, 2019
This was referenced Dec 28, 2019
@1ec5 1ec5 mentioned this pull request Jan 7, 2020
@1ec5 1ec5 modified the milestones: v1.0.0, v0.31.0 (v0.40) Apr 14, 2020
This was referenced Aug 28, 2020
@1ec5 1ec5 mentioned this pull request Nov 3, 2020
2 tasks
@1ec5 1ec5 mentioned this pull request Jun 21, 2023
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 objective-c 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 Please refactor your unwrapping optionals Adopt JSONDecoder
2 participants