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

Remove leftovers from stop-based functions #11247

Closed
jfirebaugh opened this issue Feb 19, 2018 · 5 comments
Closed

Remove leftovers from stop-based functions #11247

jfirebaugh opened this issue Feb 19, 2018 · 5 comments
Assignees
Labels
Core The cross-platform C++ core, aka mbgl refactor

Comments

@jfirebaugh
Copy link
Contributor

We're intending to take advantage of the fact that the upcoming releases of the iOS and Android SDKs are semver-major versions in order to introduce expressions as a complete replacement for the prior stop-based function functionality, and remove the stop-based APIs entirely. (Existing styles which use stop functions will be transparently converted to equivalent expressions when loaded.)

We should review the iOS and Android SDK APIs now that expressions have landed, and confirm that all stop-based function APIs have been removed.

As a second confirmation that we haven't left any lingering APIs which we'd be unable to remove until the next semver-major release, we should make the relevant core code private. This will allow us to refactor the core code at our leisure. (Among other things, such core refactoring may allow us to decrease the binary size.)

@jfirebaugh
Copy link
Contributor Author

I pushed release-boba...11247 to test this out.

In core, the stringify functions for *Function need to replace stringification of the stops with serialization (from #11156) of the expression. (This should be fine; for *Function I believe stringify is needed only for groupByLayout.)

The whole Function class hierarchy still exists in the Android SDK. @tobrun is the plan to remove this?

@jfirebaugh jfirebaugh added this to the android-v6.0.0 milestone Feb 19, 2018
@jfirebaugh jfirebaugh added Core The cross-platform C++ core, aka mbgl Android Mapbox Maps SDK for Android labels Feb 19, 2018
@tobrun
Copy link
Member

tobrun commented Feb 20, 2018

The whole Function class hierarchy still exists in the Android SDK. @tobrun is the plan to remove this?

Yes, going to tackle this with accessors in #10721, syncing with Ivo on this today.

@tobrun tobrun added the release blocker Blocks the next final release label Feb 23, 2018
@jfirebaugh jfirebaugh removed this from the android-v6.0.0 milestone Mar 14, 2018
@jfirebaugh jfirebaugh added refactor and removed release blocker Blocks the next final release Android Mapbox Maps SDK for Android labels Mar 14, 2018
@jfirebaugh jfirebaugh changed the title Confirm that SDK APIs don't have any leftovers from stop-based functions Remove leftovers from stop-based functions Mar 14, 2018
@jfirebaugh
Copy link
Contributor Author

I updated release-boba...11247 showing that we can now do this cleanup at our leisure; i.e. on master sometime after release-boba is merged back.

@jfirebaugh
Copy link
Contributor Author

#11509 wound up removing the stops member, but there's still significant cleanup work we could do here.

@jfirebaugh
Copy link
Contributor Author

jfirebaugh commented Jun 27, 2018

This is currently looking like the number one thing we could do to reduce binary size. If we can remove all the *Stops templates and convert JSON directly to expressions, it will probably produce a substantial size savings.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl refactor
Projects
None yet
Development

No branches or pull requests

2 participants