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

Add rendering of man_made=wastewater_plant, water_works #3376

Merged
merged 3 commits into from
Sep 13, 2018
Merged

Add rendering of man_made=wastewater_plant, water_works #3376

merged 3 commits into from
Sep 13, 2018

Conversation

Adamant36
Copy link
Contributor

Fixes #717
This pull request adds rendering of man_made=wastewater_plant and man_made=water_works in the industrial landuse color. Its my first pull request of this type. So, I might of added extra things that didn't need to be there or something. If so, feel free to let me know and I will correct it. I'm also not sure if the zoom levels are correct or not. I'll fix those also if need be.

wastewater plant
https://www.openstreetmap.org/#map=17/37.94864/-122.49576 (way)
wastewater_plant way
https://www.openstreetmap.org/#map=19/37.89582/-122.52669 (node)
wastewater_plant node
water works
https://www.openstreetmap.org/#map=19/38.60624/-121.52483 (way)
water_works way
water works node
water_works node

@Tomasz-W Tomasz-W mentioned this pull request Sep 2, 2018
26 tasks
@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 2, 2018

I like this idea.

@Adamant36
Copy link
Contributor Author

Me to. At least it didn't already have a PR for it :)

@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 4, 2018

There's a conflict to resolve caused by merging #3327.

@Adamant36
Copy link
Contributor Author

Thanks for pointing it out. I resolved it.

@HolgerJeromin
Copy link
Contributor

This was dicussed in the past, as landuse=industrial should be tagged in many cases...

@@ -40,12 +40,12 @@
[feature = 'tourism_alpine_hut'][zoom >= 13],
[feature = 'tourism_wilderness_hut'][zoom >= 13],
[feature = 'amenity_shelter'][zoom >= 16] {
marker-file: url('symbols/shelter.svg');
marker-file: url('symbols/amenity/shelter.svg');
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these file moves from you or a merge / git artefact?
This should be done in a seperate PR.

@Adamant36
Copy link
Contributor Author

@HolgerJeromin, thanks for noticing it. it was from a different, earlier PR. I don't know why it did that, as they are on different branches and I don't remember having to commit all that stuff again when I did this one. Its showing up here as a commit though and its in the wastewater branch to when it shouldn't be. So I don't know. the whole thing is rather irritating. How do I go about cleaning it up properly?

@Adamant36
Copy link
Contributor Author

Adamant36 commented Sep 5, 2018

@HolgerJeromin, I think I fixed it. Feel free to let me know if there is anything else wrong with it. Also, although it was discussed in the past there wasn't a consensus on it and still 5 years later only 10% of them are tagged with landuse=industrial. Which is the exact same percent as back when the argument to not render them based on that was made. So whatever was or wasn't decided here, its clearly not what is happening with actual tagging. So I think they are worth rendering on their own. Instead of relying on people to tag them as landuse=industrial when its clearly not going to happen.

@kocio-pl
Copy link
Collaborator

Please wrap some long lines, but that's all - my test shows that it works OK. You have also made valid point about landuse tagging.

There are a lot of such objects and it's good that they will start to be visible at last:

taghistory 44

@Adamant36
Copy link
Contributor Author

@kocio-pl, I wrapped some lines. So it should be good to go now.

@kocio-pl kocio-pl merged commit 2e21690 into gravitystorm:master Sep 13, 2018
@kocio-pl
Copy link
Collaborator

Thanks! Only about 10-15% of such objects have landuse tagged after ~10 years, so this is nice (and realistic) extension of the map.

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.

Add rendering for man_made=wastewater_plant
3 participants