[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL #9918

fabian-guerra · 2017-09-05T20:16:01Z

Deprecates styles according to the latest spec.

friedbunny · 2017-09-05T21:40:14Z

platform/darwin/src/MGLStyle.mm

@@ -134,6 +132,26 @@ + (NSURL *)emeraldStyleURL {
    return MGLStyleURL_emerald;
 }

+// Emerald is no longer getting new versions as a default style, so the current version is hard-coded here.


These comments say “Emerald”.

1ec5 · 2017-09-06T17:49:12Z

platform/darwin/src/MGLStyle.h

 */
-+ (NSURL *)trafficDayStyleURLWithVersion:(NSInteger)version;
+ (NSURL *)trafficDayStyleURL __attribute__((deprecated("Create an NSURL object with the string “mapbox://styles/mapbox/traffic-day-v2”.")));


This change renames the selector from -trafficDayStyleURLWithVersion: to -trafficDayStyleURL. This is a backwards-incompatible change. Consider retaining the original method name but deprecating it nonetheless.

@1ec5 I'm deprecating + (NSURL *)trafficDayStyleURL; and deleting trafficDayStyleURLWithVersion:(NSInteger)version is that ok or should I keep the later?

We shouldn’t delete anything without a major version bump. So we’ll need to keep both symbols and mark them both as deprecated.

friedbunny · 2017-09-13T19:06:42Z

platform/macos/app/MapDocument.m

@@ -926,8 +914,6 @@ - (NSUInteger)indexOfStyleInToolbarItem {
        [MGLStyle darkStyleURL],
        [MGLStyle satelliteStyleURL],
        [MGLStyle satelliteStreetsStyleURL],
-        [MGLStyle trafficDayStyleURL],
-        [MGLStyle trafficNightStyleURL],


For consistency with iosapp, we might as well keep the styles-as-URLs here, too.

1ec5 · 2017-09-14T17:22:17Z

platform/macos/app/MapDocument.m

-            styleURL = [MGLStyle trafficDayStyleURL];
-            break;
-        case 8:
-            styleURL = [MGLStyle trafficNightStyleURL];


The menu items themselves are defined in MapDocument.xib (for the toolbar) and MainMenu.xib (for the View menu). Removing the backing code without removing the UI elements leads to the assertion below when interacting with the UI elements.

1ec5 · 2017-09-14T19:10:18Z

platform/macos/app/MapDocument.m

+            styleURL = [NSURL URLWithString:@"mapbox://styles/mapbox/traffic-day-v2"];
+            break;
+        case 8:
+            styleURL = [NSURL URLWithString:@"mapbox://styles/mapbox/traffic-night-v2"];


To clarify, it’s fine to remove the deprecated styles from macosapp – after all, Emerald isn’t listed – but you need to remove them completely. Otherwise, any inconsistencies between this code and the XIBs could result in a crash.

Removed or kept, we should do the same here and in iosapp.

1ec5 · 2017-09-14T19:11:59Z

platform/darwin/test/MGLStyleTests.mm

@@ -140,7 +132,7 @@ - (void)testStyleURLDeclarations {
    NSString *styleHeader = self.stringWithContentsOfStyleHeader;

    NSError *versionedMethodError;
-    NSString *versionedMethodExpressionString = @(R"RE(^\+\s*\(NSURL\s*\*\s*\)\s*\w+StyleURLWithVersion\s*:\s*\(\s*NSInteger\s*\)\s*version\s*;)RE");
+    NSString *versionedMethodExpressionString = @(R"RE(^\+\s*\(NSURL\s*\*\s*\)\s*\w+StyleURLWithVersion\s*:\s*\(\s*NSInteger\s*\)\s*version\s*)RE");


Might be a good idea to add a \b where the ; was, to ensure that all the methods have the same parameter name (and not something like versions).

1ec5 · 2017-09-14T19:13:13Z

platform/darwin/test/MGLStyleTests.mm

@@ -99,14 +99,6 @@ - (void)testVersionedStyleURLs {
                          @(mbgl::util::default_styles::satelliteStreets.url));
    XCTAssertEqualObjects([MGLStyle satelliteStreetsStyleURLWithVersion:99].absoluteString,
                          @"mapbox://styles/mapbox/satellite-streets-v99");
-    XCTAssertEqualObjects([MGLStyle trafficDayStyleURLWithVersion:mbgl::util::default_styles::trafficDay.currentVersion].absoluteString,
-                          @(mbgl::util::default_styles::trafficDay.url));


We should continue to test these versioned methods as long as they’re part of the API. To work around the deprecation warnings, use pragmas.

friedbunny

LGTM, but I defer to @1ễc5.

1ec5 · 2017-09-15T18:44:57Z

platform/darwin/src/MGLStyle.mm

+}
+
+ (NSURL *)trafficDayStyleURLWithVersion:(NSInteger)version {
+    return [MGLStyle trafficDayStyleURL];


[MGLStyle trafficDayStyleURLWithVersion:1] ends up returning Traffic Day v2. This could be a breaking change for any developers calling this method.

@1ec5 I may misunderstood this but is it possible return a versioned style now that is deprecated? in the docs I wrote https://github.com/mapbox/mapbox-gl-native/pull/9918/files#diff-1d168de285fc4ba01ce48c4236f0b3ceR298

-trafficDayStyleURLWithVersion: should return whatever version is specified, like it always has. -trafficDayStyleURL should return version 2. Both methods should be deprecated but should continue to work like they always have.

… in MGLStyleTests.

1ec5

Interestingly, it looks like we were never testing the unversioned traffic methods in the first place.

1ec5 · 2017-09-15T20:58:15Z

platform/darwin/src/MGLStyle.h


 /**
- Returns the URL to the given version of the
+ Returns the URL to to the version 2 of the


Typo: “to to”.

….1.4 * release-ios-v3.6.0-android-v5.1.0: (36 commits) [android] [auto] Update properties to version 5.1.4 in preparation for build. [android] - latLngBounds test [android] - update changelog for 5.1.4 release. bump MAS version number to 2.2.3 (mapbox#9901) [android] fix is download complete (a download is complete when count and required resources match and the download state is inactive) (mapbox#9913) [android] - avoid adding duplicate points to bounds [android] Clear out mapCallback's OnMapReadyCallbacks on onDestroy [android] - harden offline region deletion Do not check connection if it is local request [android] - disable rotation gesture when pinch zooming macos-v0.5.1 [ios] Bump podspec to 3.6.4 (mapbox#10059) [android, ios, macos] Updated translations [core] make sure tiles are not treated as complete until all worker operations completed [core] keep tiles renderable even if a subsequent error occurs [ios] Be sure to get a BOOL value for nullable dict keys iosv3.6.3 podspec bump (mapbox#10008) [ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL (mapbox#9918) [ios] Use constraints to manage ornament view placement (again) (mapbox#9995) [ios] Canned spam coming from Transifex ... # Conflicts: # platform/android/MapboxGLAndroidSDK/proguard-rules.pro # platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapGestureDetector.java

fabian-guerra self-assigned this Sep 5, 2017

fabian-guerra requested a review from 1ec5 September 5, 2017 20:16

fabian-guerra added this to the ios-v3.6.3 milestone Sep 5, 2017

friedbunny reviewed Sep 5, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-update-core-styles branch from 9b2cfcf to 3934c78 Compare September 5, 2017 22:03

1ec5 suggested changes Sep 6, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-update-core-styles branch 2 times, most recently from 4474f33 to 6738042 Compare September 7, 2017 23:24

kkaefer added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Sep 11, 2017

friedbunny reviewed Sep 13, 2017

View reviewed changes

1ec5 suggested changes Sep 14, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-update-core-styles branch from 6738042 to efafdfa Compare September 14, 2017 17:48

1ec5 reviewed Sep 14, 2017

View reviewed changes

fabian-guerra added 2 commits September 15, 2017 14:02

[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL

2a781e7

[ios, macos] Update changelogs.

e6fb582

fabian-guerra force-pushed the fabian-update-core-styles branch from efafdfa to d392203 Compare September 15, 2017 18:09

friedbunny approved these changes Sep 15, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-update-core-styles branch from d392203 to 5a4e0c0 Compare September 15, 2017 18:35

1ec5 reviewed Sep 15, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-update-core-styles branch from 5a4e0c0 to 0114484 Compare September 15, 2017 20:14

[ios, macos] Update trafficDayStyleURL and trafficNightStyleURL tests…

d369f8d

… in MGLStyleTests.

fabian-guerra force-pushed the fabian-update-core-styles branch from 0114484 to d369f8d Compare September 15, 2017 20:16

1ec5 approved these changes Sep 15, 2017

View reviewed changes

fabian-guerra merged commit b2e33a4 into release-ios-v3.6.0-android-v5.1.0 Sep 15, 2017

fabian-guerra deleted the fabian-update-core-styles branch September 15, 2017 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL #9918

[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL #9918

fabian-guerra commented Sep 5, 2017 •

edited

Loading

friedbunny Sep 5, 2017

1ec5 Sep 6, 2017

fabian-guerra Sep 6, 2017

1ec5 Sep 6, 2017

friedbunny Sep 13, 2017

1ec5 Sep 14, 2017 •

edited

Loading

1ec5 Sep 14, 2017

friedbunny Sep 14, 2017

1ec5 Sep 14, 2017

1ec5 Sep 14, 2017

friedbunny left a comment

1ec5 Sep 15, 2017

fabian-guerra Sep 15, 2017

1ec5 Sep 15, 2017

1ec5 left a comment

1ec5 Sep 15, 2017

[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL #9918

[ios, macos] Deprecate trafficDayStyleURL and trafficNightStyleURL #9918

Conversation

fabian-guerra commented Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra commented Sep 5, 2017 •

edited

Loading

1ec5 Sep 14, 2017 •

edited

Loading