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

Add number format expression #15424

Merged
merged 3 commits into from
Sep 2, 2019
Merged

Add number format expression #15424

merged 3 commits into from
Sep 2, 2019

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Aug 20, 2019

Closes #14840

@chloekraw do we want to include this in release-queso or release-ristretto?

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Aug 20, 2019
@tobrun tobrun requested review from LukasPaczos and jmalanen August 20, 2019 14:31
@tobrun tobrun self-assigned this Aug 20, 2019
LukasPaczos
LukasPaczos reviewed Aug 20, 2019
View reviewed changes
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

Looks good!

Could you add an instrumentation test that takes a variable expression as a parameter, instead of an expression that evaluates to a constant?

@chloekraw
Copy link
Contributor

Thanks for the ping @tobrun. Let's release in ristretto.

cc @maps-ios, we should try to get number-format bindings in ristretto as well.

@chloekraw chloekraw added this to the release-ristretto milestone Aug 20, 2019
@chloekraw
Copy link
Contributor

chloekraw commented Aug 20, 2019 •
edited
Loading

Ah, sorry, Darwin bindings are already complete. I was confusing this with aggregated cluster properties. Still, let's release in ristretto.

@tobrun tobrun force-pushed the tvn-number-format-expression branch from 14278c9 to 3321bde Compare August 21, 2019 11:55
@tobrun tobrun added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Aug 21, 2019
@tobrun
Copy link
Member Author

tobrun commented Aug 21, 2019

Adding do not merge label, waiting for branch cut of release-queso

@tobrun tobrun removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Aug 21, 2019
LukasPaczos
LukasPaczos approved these changes Aug 26, 2019
View reviewed changes
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

🚢

@chloekraw chloekraw added the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 26, 2019
@tobrun tobrun removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 29, 2019
@chloekraw
Copy link
Contributor

@tobrun before merging, could we add some more detail to the changelog about what the number-format expression does?

@tobrun tobrun merged commit 0fa5944 into master Sep 2, 2019
@tobrun tobrun deleted the tvn-number-format-expression branch September 2, 2019 12:48
@tobrun tobrun mentioned this pull request Sep 6, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@LukasPaczos LukasPaczos LukasPaczos approved these changes

@jmalanen jmalanen Awaiting requested review from jmalanen

Assignees

@tobrun tobrun

Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Milestone
release-ristretto
Development

Successfully merging this pull request may close these issues.
3 participants
@tobrun @chloekraw @LukasPaczos