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

Remove expression API for non-dds properties #11816

Closed
wants to merge 1 commit into from

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented May 2, 2018

This PR fixes the issue shown in #11790. We were generating expression APIs for all different style/paint properties while we should only do this when a property is data driven.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label May 2, 2018
@tobrun tobrun added this to the android-v6.1.0 milestone May 2, 2018
@tobrun tobrun self-assigned this May 2, 2018
@tobrun tobrun requested a review from anandthakker May 2, 2018 12:20
@tobrun
Copy link
Member Author

tobrun commented May 2, 2018

The build is failing of heatmap related properties not being available (not generating the expression API for it). Looking at the style spec it seems that we haven't updated the properties to be datadriven for gl-js, iOS and Android (cc @anandthakker @1ec5).

@anandthakker
Copy link
Contributor

Hm, I'm not sure if this is the right fix, because for non-data-driven properties, zoom-only expressions are still allowed. (Note that there's no type-level enforcement for this: the validity of an expression for a given style property can only be checked at runtime.)

@tobrun
Copy link
Member Author

tobrun commented May 4, 2018

Hm, I'm not sure if this is the right fix, because for non-data-driven properties, zoom-only expressions are still allowed. (Note that there's no type-level enforcement for this: the validity of an expression for a given style property can only be checked at runtime.)

Let's move forward with a different approach that handles the case in #11790 more gracefully.

@tobrun tobrun closed this May 4, 2018
@tobrun tobrun removed this from the android-v6.1.0 milestone May 4, 2018
@jfirebaugh jfirebaugh deleted the tvn-fix-dds-integration branch July 27, 2018 22:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants