Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing objects without background/outline from landcover data layer #3153

Merged
merged 1 commit into from
Mar 31, 2018

Conversation

kocio-pl
Copy link
Collaborator

A fix triggered by finding this bug: #2440 (comment).

Objects with no area/outline rendering should not be selected in the landcover data layer. It might go unnoticed most of the time - for example it's hard for me to imagine firepit area with any kind of background, like grass - but if this area is tagged also with something using landcover features, it can lead to lack of the background/outline of the second feature.

@matthijsmelissen
Copy link
Collaborator

I'm not sure if I fully understand the need for this PR?

Also, why did we have these features in the table in the first place, if we don't need them?

@kocio-pl
Copy link
Collaborator Author

It's just code cleaning. They were probably added because some similar objects were there - or maybe in a hope that soon some kind of area/outline rendering will be defined, but they are not styled currently.

This is why #2452 would be useful - I wouldn't miss/introduce them if the change was in the clearly named whitelist instead of large SQL statement inside even bigger layer.

@matthijsmelissen
Copy link
Collaborator

Ok, clear!

@matthijsmelissen
Copy link
Collaborator

I see now, for example shower was introduced in #3134.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Mar 29, 2018

Yes. On the other hand tourism=attraction is there for ~4 years at least.

@kocio-pl kocio-pl merged commit ea769f4 into gravitystorm:master Mar 31, 2018
@kocio-pl kocio-pl deleted the landcover-layer-fix branch March 31, 2018 05:24
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.

2 participants