[ios, macos] Add notes to APIs that rely on style and source ids #6886
Conversation
boundsj
added
iOS
|
@boundsj, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1ec5, @frederoni and @rmnblm to be potential reviewers. |
| @@ -177,6 +177,13 @@ static const NSInteger MGLStyleDefaultVersion = 9; | |||
| /** | |||
| Returns a layer that conforms to `MGLStyleLayer` if any layer with the given | |||
| identifier was found. | |||
|
|
|||
| @note Layer identifiers are subject to change and are not guaranteed to exist | |||
There was a problem hiding this comment.
This is an awkward place to put “subject to change”. Maybe in the next sentence, say that the default style’s layers are subject to change.
| @note Layer identifiers are subject to change and are not guaranteed to exist | ||
| accross styles or different versions of the same style. Applications that | ||
| use this API must set the style URL to an explicitly versioned style | ||
| using a convenience method like +[MGLStyle outdoorsStyleURLWithVersion:], |
There was a problem hiding this comment.
Wrap this selector in backticks to enable autolinking in jazzy.
| accross styles or different versions of the same style. Applications that | ||
| use this API must set the style URL to an explicitly versioned style | ||
| using a convenience method like +[MGLStyle outdoorsStyleURLWithVersion:], | ||
| the “Style URL” IBInspectable in `MGLMapView`, or a manually constructed |
There was a problem hiding this comment.
MGLMapView’s “Style URL” inspectable in Interface Builder
|
Additional APIs to warn about:
|
|
Before I move on to #6886 (comment) I took another pass at the copy. Please let me know what you think. |
| @@ -177,6 +177,14 @@ static const NSInteger MGLStyleDefaultVersion = 9; | |||
| /** | |||
| Returns a layer that conforms to `MGLStyleLayer` if any layer with the given | |||
| identifier was found. | |||
|
|
|||
| @note Layer identifiers are not guaranteed to exist across styles or different | |||
| versions of the same style. Applications that use this API must set the style | |||
There was a problem hiding this comment.
s/must/must first/
I think that could be a point of confusion for developers.
| versions of the same style. Applications that use this API must set the style | ||
| URL to an explicitly versioned style using a convenience method like | ||
| `+[MGLStyle outdoorsStyleURLWithVersion:]`, `MGLMapView`'s “Style URL” | ||
| inspectable in Interface Builder, or a manually constructed NSURL. This |
There was a problem hiding this comment.
Nit: put NSURL in backticks (for consistency with SDK classes that would get autolinked, even if this one doesn’t get linked).
| explicitly versioned style using a convenience method like | ||
| `+[MGLStyle outdoorsStyleURLWithVersion:]`, `MGLMapView`'s “Style URL” | ||
| inspectable in Interface Builder, or a manually constructed `NSURL`. | ||
| */ |
There was a problem hiding this comment.
Please update the macOS implementation of MGLMapView with these changes as well.
| `+[MGLStyle outdoorsStyleURLWithVersion:]`, `MGLMapView`'s “Style URL” | ||
| inspectable in Interface Builder, or a manually constructed `NSURL`. This | ||
| approach also avoids layer identifer name changes that will occur in the default | ||
| style’s layers over time. |
boundsj
force-pushed
the
boundsj-warn-about-style-ids
branch
from
692b0bb
to
4afe9f7
Compare
|
@1ec5 what do you think about this now? |
|
I think there’s still a typo in a couple places: #6886 (comment) |
boundsj
force-pushed
the
boundsj-warn-about-style-ids
branch
from
4afe9f7
to
f990b31
Compare
|
Corrected and rebased @1ec5 |
Fixes #6616 for iOS and macOS
cc @1ec5