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.

Sign up for GitHub

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

Jump to bottom

Require map SDK v5.5.0 in v0.37.x (redux) #2317

Merged
merged 1 commit into from
Jan 23, 2020
Merged

Require map SDK v5.5.0 in v0.37.x (redux) #2317

merged 1 commit into from
Jan 23, 2020

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jan 22, 2020 •
edited
Loading

This is a reprise of #2301 that disallows versions of the map SDK other than v5.5.0. In other words, if map SDK v5.5.1 comes out with a critical bug fix, projects that depend on the navigation SDK will not receive v5.5.1 unless we also release a patch release of the navigation SDK.

This change targets the v0.37.x release series.

/ref #2299
/cc @mapbox/navigation-ios @mapbox/maps-ios @mapbox/mobile-telemetry @mapbox/vision-ios

@1ec5 1ec5 added the build Issues related to builds and dependency management. label Jan 22, 2020
@1ec5 1ec5 added this to the v0.37.2 milestone Jan 22, 2020
@1ec5 1ec5 requested review from JThramer and chezzdev January 22, 2020 07:45
@1ec5 1ec5 self-assigned this Jan 22, 2020
@1ec5 1ec5 changed the title Require map SDK v5.5.0 and no other version in v0.37.x Require map SDK v5.5.0 in v0.37.x (redux) Jan 22, 2020
@1ec5 1ec5 mentioned this pull request Jan 22, 2020
Merged
1ec5
1ec5 commented Jan 22, 2020
View reviewed changes
Copy link
Contributor Author

@1ec5 1ec5 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

After this change is merged into the release-v0.37 branch, we’ll cherry-pick it into the release-v0.38 branch and release both v0.37.2 and v0.38.3.

MapboxNavigation.podspec
@@ -44,7 +44,7 @@ Pod::Spec.new do |s|
s.module_name = "MapboxNavigation"

s.dependency "MapboxCoreNavigation", "#{s.version.to_s}"
s.dependency "Mapbox-iOS-SDK", "~> 5.5.0"
s.dependency "Mapbox-iOS-SDK", "= 5.5.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CocoaPods wasn’t affected by the original issue, but I’m pinning the podspec too to ensure consistent dependencies regardless of the dependency manager used.

1ec5
1ec5 commented Jan 22, 2020
View reviewed changes
MapboxNavigation-Documentation.podspec
@@ -47,7 +47,7 @@ Pod::Spec.new do |s|

s.dependency "MapboxDirections.swift", "~> 0.30.0"
s.dependency "MapboxGeocoder.swift", "~> 0.10.0"
s.dependency "Mapbox-iOS-SDK", "~> 5.5.0"
s.dependency "Mapbox-iOS-SDK", "= 5.5.0"
s.dependency "MapboxMobileEvents", "~> 0.9.5"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we also pin MapboxMobileEvents to v0.9.5 specifically and avoid patch releases? Unlike with the map SDK, this branch is still on a pre-v1.0 release, so it wouldn’t float to v0.10.x.

/cc @rclee @alfwatt

chezzdev
chezzdev approved these changes Jan 22, 2020
View reviewed changes
Copy link
Contributor

@chezzdev chezzdev left a comment

Choose a reason for hiding this comment

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

LGTM, just a small clarification in a test.

MapboxCoreNavigationTests/CocoaPodsTest/PodInstall/Podfile.lock
@@ -1,16 +1,16 @@
PODS:
- Mapbox-iOS-SDK (5.5.0)
- MapboxCoreNavigation (0.37.0):
- MapboxCoreNavigation (0.37.1):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why the previous version should be used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CocoaPods installation test lags official releases by design. We haven’t released v0.37.2 yet, so the test can’t verify that running pod update would install v0.37.2. However, it would detect if a newer dependency would be pulled in. This is why the test has failed quite often during the v1.0 development process, as breaking changes were introduced between prereleases of MapboxDirections.

@1ec5 1ec5 requested review from julianrex and alfwatt January 22, 2020 18:35
JThramer
JThramer approved these changes Jan 22, 2020
View reviewed changes
Copy link
Contributor

@JThramer JThramer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

:shipit:

@1ec5 1ec5 merged commit 05b1ab8 into release-v0.37 Jan 23, 2020
@1ec5 1ec5 deleted the 1ec5-map-v5.5.0-strict-2299 branch January 23, 2020 07:57
@1ec5
Copy link
Contributor Author

1ec5 commented Jan 23, 2020

Cherry-picked into the release-v0.38 branch as b666161.

@1ec5 1ec5 mentioned this pull request Jan 23, 2020
Closed
@1ec5 1ec5 mentioned this pull request Jul 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JThramer JThramer JThramer approved these changes

@julianrex julianrex julianrex approved these changes

@chezzdev chezzdev chezzdev approved these changes

@alfwatt alfwatt Awaiting requested review from alfwatt

Assignees

@1ec5 1ec5

Labels
build Issues related to builds and dependency management.
Projects
None yet
Milestone
v0.37.2
Development

Successfully merging this pull request may close these issues.
4 participants
@1ec5 @JThramer @julianrex @chezzdev