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

[core] Implement 'in' expression. #16162

Merged
merged 8 commits into from
Feb 15, 2020
Merged

[core] Implement 'in' expression. #16162

merged 8 commits into from
Feb 15, 2020

Conversation

Chaoba
Copy link
Contributor

@Chaoba Chaoba commented Feb 3, 2020

Launch Checklist

Resolves #15893
This PR implements in expression and turns on the corresponding tests in expression-test.

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • apply needs changelog label if a changelog is needed (remove label when added)

@Chaoba Chaoba added Core The cross-platform C++ core, aka mbgl expressions labels Feb 3, 2020
@Chaoba Chaoba changed the title [WIP]Create in expression head file [WIP]Implement 'in' expression. Feb 4, 2020
@Chaoba Chaoba force-pushed the kl-expression-in branch 5 times, most recently from f15ba74 to 3a787c7 Compare February 6, 2020 09:34
@Chaoba Chaoba changed the title [WIP]Implement 'in' expression. [core] Implement 'in' expression. Feb 6, 2020
@Chaoba Chaoba self-assigned this Feb 6, 2020
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Outdated Show resolved Hide resolved
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
@alexshalamov alexshalamov added the needs changelog Indicates PR needs a changelog entry prior to merging. label Feb 10, 2020
@Chaoba Chaoba force-pushed the kl-expression-in branch 2 times, most recently from ee8510f to 9cb1f21 Compare February 11, 2020 09:12
@alexshalamov
Copy link
Contributor

@Chaoba could you also add equality tests for "in" expression to test/fixtures/expression_equality/?

src/mbgl/style/expression/in.cpp Outdated Show resolved Hide resolved
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
include/mbgl/style/expression/in.hpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Show resolved Hide resolved
src/mbgl/style/expression/in.cpp Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@Chaoba
Copy link
Contributor Author

Chaoba commented Feb 13, 2020

Waiting for mapbox/mapbox-gl-js#9295 , as there is a failed test case blocks now.

CHANGELOG.md Show resolved Hide resolved
@Chaoba Chaoba removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Feb 13, 2020
Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

I don't want to offer a full review of the C++ implementation but overall this looks correct to me 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@alexshalamov
Copy link
Contributor

@Chaoba FYI mapbox/mapbox-gl-js#9295

Copy link
Contributor

@alexshalamov alexshalamov left a comment

Choose a reason for hiding this comment

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

lgtm

@Chaoba Chaoba merged commit 02274e7 into master Feb 15, 2020
@Chaoba Chaoba deleted the kl-expression-in branch February 15, 2020 09:37
@tmpsantos
Copy link
Contributor

@Chaoba thanks a lot for your contribution.

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 expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement 'in' expression
5 participants