Version default style URL APIs; deprecate Emerald #4759

1ec5 · 2016-04-20T05:05:39Z

Updated default styles from v8 to v9 in the centralized default style constants used by the iOS and OS X SDKs as well as by the GLFW application. Deprecated the MGLMapView class methods in favor of new methods that take a version parameter. Replaced usage of the unversioned MGLStyle methods with the corresponding versioned methods and MGLStyleCurrentVersion to ensure consistency.

Expanded MGLStyle unit tests to also assert that MGLStyle has the right number of versioned style URL methods and that they’re all public.

Deprecate unversioned style URL methods in favor of versioned methods (Add version argument to class members of Style, deprecate older ones #4702)
Deprecate Emerald (Deprecate Emerald #4577)
Rename Hybrid to Satellite Streets
Figure out how to make MGLStyleCurrentVersion not be a trap: Version default style URL APIs; deprecate Emerald #4759 (comment)
Keep unversioned style URL methods on v8: [android] #4702 - upgrade default styles for Android #4812 (comment)
Revise style descriptions to match www.mapbox.com

Fixes #4577 and fixes #4702. Hold until the v9 styles near release.

/cc @jfirebaugh @friedbunny

friedbunny · 2016-04-20T16:40:06Z

platform/darwin/include/MGLStyle.h

+/**
+ A version number identifying the latest released version of the suite of default styles provided by Mapbox. This version number may be passed into one of the “StyleURLWithVersion” class methods of MGLStyle.
+
+ The value of this constant is current as of the date on which this SDK was published. Consult the [Mapbox Styles API documentation](https://www.mapbox.com/api-documentation/#styles) for the most up-to-date style versioning information.


Per #4760, we’ll want to HTML'ize this link.

jfirebaugh · 2016-04-20T17:51:38Z

platform/darwin/include/MGLStyle.h

+
+ The value of this constant is current as of the date on which this SDK was published. Consult the <a href="https://www.mapbox.com/api-documentation/#styles">Mapbox Styles API documentation</a> for the most up-to-date style versioning information.
+
+ @warning The value of this constant may change in a future release of the SDK. If you use any feature that depends on a specific implementation detail in a default style, you may use the current value of this constant or the underlying style URL, but do not use the constant itself. Such implementation details may change significantly from version to version.


Let me go on the record stating that I think including a MGLStyleCurrentVersion constant is a trap and that we should require people to explicitly specify a version number.

But it we do include it, we should not use the term "implementation detail" -- this suggests that we don't consider style features to be part of an API contract, when in fact the very reason we are using versions is to indicate changes to documented semantics.

I was thinking of it being used for sanity-checking more than anything. For example, a developer might want to assert in a unit test that the current version is still 9, as a reminder to upgrade the application to use the newest default style with the runtime styling API. We’d avoid using this constant anywhere else in documentation or guides. It’s sort of like how this framework (and all others) provides a MapboxVersionNumber constant just in case.

But you’re right: the way I’ve worded references to this constant in e.g. -streetsStyleURLWithVersion: could definitely lead developers to use it without reading this documentation and understanding the ramifications. If we can’t settle on a safer way to treat this constant, I agree that we should make it private.

How does 1e3b1f2 look to you? The constant is now MGLStyleDefaultVersion, with documentation that positions it as “here’s what MGLMapView chooses by default” rather than “here’s what you should pass into MGLStyle”. There are no public references to this constant; instead, documentation for each method spells out the current version “as of publication”, and a unit test verifies that it’s in sync with the constant.

tobrun · 2016-04-23T06:57:53Z

Android equivalent targeting to merge with this PR in #4812

tobrun · 2016-04-28T14:05:59Z

Android equivalent was merged with this PR in #4812

1ec5 · 2016-05-02T22:46:04Z

As long as everything checks out, I’m going to branch off the v3.2.1 tag and cherry-pick these changes into that branch (or at least everything that fits after resolving conflicts), then release v3.2.2 from that branch.

friedbunny · 2016-05-03T01:36:26Z

This looks good to me and works in practice (merged into a local v3.2.1 branch), even if the new styles aren’t available yet.

MGLStyleDefaultVersion is public, but I think the inline docs should be sufficient warning to developers about the perils of using it in production. I’m fine with explicitly not using MGLStyleDefaultVersion in examples and always hard-coding style version in code we want the general public to emulate.

friedbunny · 2016-05-03T03:18:36Z

Running make iframework, there are warnings about NSInteger casting for each style (L26-30):

⚠️  /mapbox-gl-native/platform/darwin/src/MGLStyle.mm:25:1: values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead [-Wformat]

MGL_DEFINE_STYLE(streets, streets)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

1ec5 · 2016-05-03T21:33:23Z

Running make iframework, there are warnings about NSInteger casting for each style (L26-30):

Just fixed and rebased. Thanks!

1ec5 · 2016-05-03T21:50:56Z

Cherry-picked the changes into the release-ios-v3.2.2 branch as ~~6fd80ae...d738cae~~ 6fd80ae...6f981e9. I omitted ~~0845158~~ da422ff because only the iOS SDK will be built from this branch and there were lots of Android conflicts that I didn’t quite follow.

Updated default styles from v8 to v9. Deprecated the MGLMapView class methods in favor of new methods that take a version parameter. Deprecated Emerald outright in favor of Outdoors. Replaced usage of the unversioned MGLStyle methods with the corresponding versioned methods and MGLStyleCurrentVersion to ensure consistency. Expanded MGLStyle unit tests to also assert that MGLStyle has the right number of style URL methods and that they’re all public. Linked the OS X SDK unit test bundle to libmbgl-core.a. Removed an unnecessary dependency on osxapp. Replaced Emerald with Outdoors in iosapp and osxapp. Fixes the iOS and OS X side of #4577 and #4702.

…nition in favour versioned styling methods.

The style ID has also changed, but the unversioned method will continue to point to the old v8 style ID.

Renamed MGLStyleCurrentVersion to MGLStyleDefaultVersion to emphasize the constant’s role as an indicator of the default version used by SDK classes, which may be behind depending on release schedules. In the documentation for each style URL method, include the actual version number instead of the constant. In unit tests, verify that this documentation is in sync with the constant. Link each style URL method’s documentation to a forthcoming webpage on mapbox.com that describes the style in rich detail.

Revised style descriptions to be more consistent with how the styles are described on www.mapbox.com.

1ec5 self-assigned this Apr 20, 2016

1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Core The cross-platform C++ core, aka mbgl labels Apr 20, 2016

1ec5 added this to the ios-v3.2.1 milestone Apr 20, 2016

1ec5 changed the title ~~Version default style URL APIs; deprecated Emerald~~ Version default style URL APIs; deprecate Emerald Apr 20, 2016

This was referenced Apr 20, 2016

Deprecate Emerald #4577

Closed

Add version argument to class members of Style, deprecate older ones #4702

Closed

friedbunny mentioned this pull request Apr 20, 2016

Update for v9 styles mapbox/ios-sdk-examples#13

Closed

5 tasks

1ec5 added ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold ✓ ready for review labels Apr 20, 2016

friedbunny reviewed Apr 20, 2016
View reviewed changes

1ec5 force-pushed the 1ec5-default-styles-4577 branch from 13851c2 to 61b8409 Compare April 20, 2016 17:38

jfirebaugh reviewed Apr 20, 2016
View reviewed changes

1ec5 force-pushed the 1ec5-default-styles-4577 branch from 61b8409 to c2dee9b Compare April 23, 2016 04:18

tobrun mentioned this pull request Apr 23, 2016

[android] #4702 - upgrade default styles for Android #4812

Merged

1ec5 removed the ✓ ready for review label Apr 26, 2016

1ec5 force-pushed the 1ec5-default-styles-4577 branch 3 times, most recently from 3edf00d to 1571a42 Compare May 2, 2016 21:29

1ec5 added Android Mapbox Maps SDK for Android ✓ ready for review and removed ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels May 2, 2016

friedbunny mentioned this pull request May 3, 2016

Update to v9 styles and iOS v3.2.2 mapbox/ios-sdk-examples#14

Merged

5 tasks

1ec5 force-pushed the 1ec5-default-styles-4577 branch from 1e3b1f2 to fac8530 Compare May 3, 2016 21:30

1ec5 added ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold and removed ✓ ready for review labels May 3, 2016

1ec5 and others added 6 commits May 6, 2016 10:57

[android] #4702 - upgrade default styles, deprecate Style string defi…

da422ff

…nition in favour versioned styling methods.

[ios, osx] Pin unversioned style URL APIs to v8 styles

4a112c0

[ios, osx] Renamed Hybrid to Satellite Streets

5ccb434

The style ID has also changed, but the unversioned method will continue to point to the old v8 style ID.

[ios, osx] Revised style descriptions

5035dc8

Revised style descriptions to be more consistent with how the styles are described on www.mapbox.com.

1ec5 force-pushed the 1ec5-default-styles-4577 branch from 58283e7 to 5035dc8 Compare May 6, 2016 18:07

1ec5 merged commit 5035dc8 into master May 6, 2016

1ec5 deleted the 1ec5-default-styles-4577 branch May 6, 2016 18:37

1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label May 6, 2016

1ec5 mentioned this pull request Sep 10, 2016

Pin to style version mapbox/ViewAnnotationDemoiOS#1

Merged

1ec5 mentioned this pull request Oct 6, 2016

Clearly document that any runtime styling API risks breaking changes #6616

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Version default style URL APIs; deprecate Emerald #4759

Version default style URL APIs; deprecate Emerald #4759

1ec5 commented Apr 20, 2016 •

edited

Loading

friedbunny Apr 20, 2016

jfirebaugh Apr 20, 2016

1ec5 Apr 20, 2016

1ec5 May 2, 2016

tobrun commented Apr 23, 2016

tobrun commented Apr 28, 2016 •

edited

Loading

1ec5 commented May 2, 2016

friedbunny commented May 3, 2016 •

edited

Loading

friedbunny commented May 3, 2016 •

edited

Loading

1ec5 commented May 3, 2016

1ec5 commented May 3, 2016 •

edited

Loading


		The value of this constant is current as of the date on which this SDK was published. Consult the <a href="https://www.mapbox.com/api-documentation/#styles">Mapbox Styles API documentation</a> for the most up-to-date style versioning information.

		@warning The value of this constant may change in a future release of the SDK. If you use any feature that depends on a specific implementation detail in a default style, you may use the current value of this constant or the underlying style URL, but do not use the constant itself. Such implementation details may change significantly from version to version.

Version default style URL APIs; deprecate Emerald #4759

Version default style URL APIs; deprecate Emerald #4759

Conversation

1ec5 commented Apr 20, 2016 • edited Loading

friedbunny Apr 20, 2016

Choose a reason for hiding this comment

jfirebaugh Apr 20, 2016

Choose a reason for hiding this comment

1ec5 Apr 20, 2016

Choose a reason for hiding this comment

1ec5 May 2, 2016

Choose a reason for hiding this comment

tobrun commented Apr 23, 2016

tobrun commented Apr 28, 2016 • edited Loading

1ec5 commented May 2, 2016

friedbunny commented May 3, 2016 • edited Loading

friedbunny commented May 3, 2016 • edited Loading

1ec5 commented May 3, 2016

1ec5 commented May 3, 2016 • edited Loading

1ec5 commented Apr 20, 2016 •

edited

Loading

tobrun commented Apr 28, 2016 •

edited

Loading

friedbunny commented May 3, 2016 •

edited

Loading

friedbunny commented May 3, 2016 •

edited

Loading

1ec5 commented May 3, 2016 •

edited

Loading