Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add Clarification for modifying ShapeCollections #7683

Closed
wants to merge 2 commits into from
Closed

Add Clarification for modifying ShapeCollections #7683

wants to merge 2 commits into from

Conversation

nitrag
Copy link
Contributor

@nitrag nitrag commented Jan 11, 2017

Spinoff task from #7622

@1ec5

@mention-bot
Copy link

@nitrag, thanks for your PR! By analyzing this pull request, we identified @1ec5, @boundsj and @friedbunny to be potential reviewers.

@@ -116,11 +116,14 @@ MGL_EXPORT

/**
The contents of the source. A shape can represent a GeoJSON geometry, a
feature, or a collection of features.
feature, or a collection of features.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: trailing whitespace.


If the receiver was initialized using `-initWithIdentifier:URL:options:`, this
property is set to `nil`. This property is unavailable until the receiver is
passed into `-[MGLStyle addSource:]`.

You can get/set the shapes within a collection via this property. Setting
this property must be performed on the application's main thread.
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the setter isn’t background-thread-safe, I’d be surprised if the getter were completely thread-safe. Best to say that this property must be accessed on the main thread and leave it at that.

@1ec5
Copy link
Contributor

1ec5 commented Jan 11, 2017

By the way, we’re developing v3.4.x on the release-ios-v3.4.0 branch, while master is for v3.5.0 and beyond. We’re merging back to master approximately once per beta. Since this is such a small change, I can cherry-pick it once it lands, but you may find it more convenient to start changes on the release branch until v3.4.0 goes out the door.

@1ec5 1ec5 added documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Jan 11, 2017
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Jan 11, 2017
@nitrag
Copy link
Contributor Author

nitrag commented Jan 11, 2017

Learned my lesson with source branch this go around, will do for next time. Thanks!

@1ec5
Copy link
Contributor

1ec5 commented Jan 11, 2017

The master branch is protected, so I can’t merge this PR until the bots pass. However, this repository’s CI bots only run for branches pushed to mapbox/mapbox-gl-native, not a fork.

Ordinarily, I’d get around this issue by pushing an identical branch to this repository, using the same name that you used on your fork; GitHub would use that branch for this PR, and the bots would start building the changes without a problem. Unfortunately, you requested to pull from your fork’s master branch, and I can’t push a new branch named “master”. So I pushed a new PR, #7687, targeted at the release-ios-v3.4.0 branch.

In the future, when you create a PR, please consider pushing your changes to a branch of your fork. That’ll make it clearer which changes are part of the PR and much easier to merge your changes into master. Thanks!

@1ec5 1ec5 closed this Jan 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants