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

Unify code of power=tower and power=pole #3561

Merged
merged 4 commits into from
May 21, 2019
Merged

Unify code of power=tower and power=pole #3561

merged 4 commits into from
May 21, 2019

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Dec 10, 2018

Fixes #1179

Changes proposed in this pull request:

  • Merge layers power-towers and power-poles in project.mml
  • Adjust modifications in power.mss

Test rendering with links to the example places:
there is no rendering differences
https://www.openstreetmap.org/#map=17/46.17629/6.23976
z=15
power_towers_z15
z=16
power_towers_z16
z=17
power_towers_z17

project.mml Outdated Show resolved Hide resolved
Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

This needs an ORDER BY to get the ordering right.

If you feel comfortable with git, go back one commit and work from there, otherwise don't worry about which order you write things within the WHERE clause.

project.mml Outdated
FROM planet_osm_point
WHERE power = 'tower'
WHERE power = 'pole' OR power = 'tower'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs an ORDER BY clause to get the correct priority when a pole and tower are nearby.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the syntax to include ORDER BY condition. Is it correct ?

@kocio-pl
Copy link
Collaborator

@jragusa What do plan to do with this code, do you want to make the requested changes?

@jragusa
Copy link
Contributor Author

jragusa commented Dec 29, 2018

yes, I'm working on it.

@kocio-pl kocio-pl changed the title Unify code of power=tower and power=pole [WIP] Unify code of power=tower and power=pole Jan 3, 2019
@imagico
Copy link
Collaborator

imagico commented Apr 6, 2019

@jragusa - do you still intend to complete this? The current code does not work because of

power
CASE power

Also note if you don't use the prio column anywhere else you don't need it, you can just inline the CASE in the ORDER BY (like done in many of the road queries for example).

@jragusa
Copy link
Contributor Author

jragusa commented May 8, 2019

@imagico I have modified according to your remark. What do you think ?

@imagico imagico changed the title [WIP] Unify code of power=tower and power=pole Unify code of power=tower and power=pole May 8, 2019
Copy link
Collaborator

@imagico imagico 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 now.

@imagico imagico dismissed pnorman’s stale review May 21, 2019 20:02

has been addressed

@imagico imagico merged commit 728032e into gravitystorm:master May 21, 2019
sommerluk pushed a commit to sommerluk/openstreetmap-carto that referenced this pull request Aug 5, 2019
* unify code of power=tower and power=pole

* invert pole and tower

* add ORDER condition

* fix ORDER BY
sommerluk pushed a commit to sommerluk/openstreetmap-carto that referenced this pull request Aug 5, 2019
* unify code of power=tower and power=pole

* invert pole and tower

* add ORDER condition

* fix ORDER BY
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.

Merge power-towers and power-poles
4 participants