Skip to content

Conversation

@migurski
Copy link
Contributor

@migurski migurski commented Dec 26, 2025

Migrate from plain Java conditionals to MultiExpression rules when applying POI kind and kindDetail. Removes some dense logic from processOsm(), passes all existing tests with no changes required.

@migurski migurski marked this pull request as ready for review December 27, 2025 21:19
Copy link
Collaborator

@wipfli wipfli left a comment

Choose a reason for hiding this comment

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

Thanks for this pull request. At first sight it looks good. Do you also plan to move the minZoom to the MultiExpressions?

Comment on lines 77 to 90
Expression.or(
with("landuse", "forest"),
Expression.and(with("boundary", "national_park"), with_operator_usfs),
Expression.and(
with("boundary", "national_park"),
with("protect_class", "6"),
with("protection_title", "National Forest")
),
Expression.and(
with("boundary", "protected_area"),
with("protect_class", "6"),
with_operator_usfs
)
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have not though about this use case for the with function. But if it works I guess it is very convenient! cc @msbarry

@migurski
Copy link
Contributor Author

Thanks for this pull request. At first sight it looks good. Do you also plan to move the minZoom to the MultiExpressions?

Yes, I have a followup PR in the works for zooms but it’s much larger so I wanted to be sure that the basic ideas in this one are correct before proposing it.

@migurski migurski requested a review from wipfli December 28, 2025 17:40
@sonarqubecloud
Copy link

@bdon bdon merged commit bf1f200 into protomaps:main Dec 29, 2025
6 checks passed
@migurski migurski deleted the migurski/migrate-poi-kinds-to-rules branch December 29, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants