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

Implement MGLForegroundStyleLayer.sourceIdentifier #7570

Merged
merged 2 commits into from
Jan 3, 2017

Conversation

frederoni
Copy link
Contributor

@frederoni frederoni commented Jan 2, 2017

Fixes #7557

This PR adds a sourceIdentifier getter for all concrete subclasses of MGLForegroundStyleLayer.

@frederoni frederoni added crash iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling labels Jan 2, 2017
@frederoni frederoni added this to the ios-v3.4.0 milestone Jan 2, 2017
@frederoni frederoni self-assigned this Jan 2, 2017
@mention-bot
Copy link

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

@jfirebaugh
Copy link
Contributor

#7557 claims that the underlying core classes contain getters and setters for source ID, but in fact setters did not exist and are being added in this PR. Changing the source ID is an operation that's more complex than simply updating the data member in the impl. Beyond that, the source tiles will need to be reparsed, and there are places in core where layers are grouped by source that will need to be updated. We'll need to add test cases to the test-suite, and port this feature to GL JS. Overall, adding a setter may turn out to be a rather complex change. Are we sure it's necessary, especially at such a late date in the release cycle?

@frederoni
Copy link
Contributor Author

frederoni commented Jan 2, 2017

I can see some convenient use cases for it but it's not an absolute necessity.
Let's drop the changes in core and keep the getter in order to fix the crash in #7557

@1ec5
Copy link
Contributor

1ec5 commented Jan 2, 2017

My mistake: we should make the SDK property read-only. Sorry for the confusion.

@frederoni frederoni force-pushed the fred-source-identifier-7557 branch from b4dae74 to 795a5cb Compare January 3, 2017 09:13
@frederoni frederoni requested a review from 1ec5 January 3, 2017 09:17
@frederoni frederoni merged commit 1aab26e into release-ios-v3.4.0 Jan 3, 2017
@frederoni frederoni deleted the fred-source-identifier-7557 branch January 3, 2017 09:25
@frederoni frederoni restored the fred-source-identifier-7557 branch January 3, 2017 09:27
@jfirebaugh jfirebaugh deleted the fred-source-identifier-7557 branch January 5, 2017 00:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crash 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.

4 participants