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 support of amenity=vending_machine #3601

Merged
merged 1 commit into from Jan 4, 2019
Merged

Add support of amenity=vending_machine #3601

merged 1 commit into from Jan 4, 2019

Conversation

jragusa
Copy link
Contributor

@jragusa jragusa commented Dec 26, 2018

Fixes #1561
Fixes #2541

Changes proposed in this pull request:

  • display amenity=vending_machine when vending=public_transport_tickets/parking_tickets/excrement_bags with dedicated icons in @amenity-brown colour
  • only render label for vending=public_transport_tickets with operator tag

Test rendering with links to the example places:
vending=public_transport_tickets
https://www.openstreetmap.org/node/3735097197
before
public_transport_before
after
public_transport_after_brownlabel

vending=parking_tickets
https://www.openstreetmap.org/#map=19/46.19375/6.23334
before
parking_ticket_before
after
parking_ticket_after

vending=excrement_bags
https://www.openstreetmap.org/node/4461667570
before
excrement_before
after
excrement_after

@sommerluk
Copy link
Collaborator

Hi. I’ve taken a look to this PR. The query seems okay. The zoom levels however should be

  • excrement_bags z21
  • parking_tickets z20
  • public_transport_tickets z20

@kocio-pl
Copy link
Collaborator

Well, they are the same size as ATM or post box and our primary deployment target does not support z20+.

@jragusa
Copy link
Contributor Author

jragusa commented Dec 27, 2018

I agree with @sommerluk and they should be moved to z20+ once these zoom levels will be supported

@sommerluk
Copy link
Collaborator

they are the same size as ATM or post box

At least for excrement bags, they are smaller. Anyway I do not follow the argument of size. I do not think either that it is an important use case for our general-purpose map to find vending machines for excrement bags.

our primary deployment target does not support z20+.

This should not influence our design decisions. If z19 as maximal zoom level is considered too low, this should be discussed with the maintainers of the rendering servers. By the way: We have yet some rules for z20+ nevertheless in our code.

@sommerluk
Copy link
Collaborator

once these zoom levels will be supported

Well, they are yet supported by our style. Indeed, we have yet some rules for z20+, and we can easily add more.

The maintainers of the openstreetmap.org website however decide independently from us which zoom levels they want to render.

@kocio-pl
Copy link
Collaborator

I do not think either that it is an important use case for our general-purpose map to find vending machines for excrement bags.

It is important for general purpose style to give general feedback it is there, finding things is not the primary goal. If you need to find something important to you personally, you have specialized services for that, for example:

http://openpoimap.org/?map=amenity&zoom=16&lat=51.52344&lon=-0.13293&layers=B00TFFFFFFFFFFFFFFFFFFFFFFFFFFFFF

@Adamant36
Copy link
Contributor

Personally, I think excrement bags are a little specialized, but they are also a good highlight of the unique capabilities of OSM compared to other maps. Which is one of the goals of the style.

Also, instead of trolling with the comments about how things you dont like should be rendered on z20, when you know z20 doesnt exist and its not going to, just be honest and say you dont think the thing should be rendered. Otherwise, its just massively useless strawman.

@sommerluk
Copy link
Collaborator

trolling with the comments

I don’t think we are “trolling” here. Please be more polite when discussing in this Github repository.

@matkoniecz
Copy link
Contributor

Also, instead of trolling with the comments about

I see no trolling here. Please, avoid accusations as substitute for arguments. It is against general etiquette, counterproductive to encouraging other to contribute and against our code of conduct.

@Adamant36
Copy link
Contributor

@sommerluk, I dont mean trolling in a derogatory sense. Its just not productive to give @kocio-pl slack for his size rule or to talk about how things are/should be rendered at z20 when theres no z20. Its already clear you and other people arent fans of the size rule. It doesnt help to continue critizing it at every chance though.

Both that and z20 are off topic. Go read the code of conduct file about staying on topic and respecting others opinions. As I said, If you dont like the icon, just say so. Its fine if you dont think its appropriate for the map. If you think there should be a z20 where these things can be rendered, thats fine. Go talk to the site admins, but is saying it should be added at a none existent rendering level is useless. Useless off topic criticism is trolling. Thats my opinion.

That being said, I wont use the word again if your sensitive about it. It wasnt my intention to hurt your feelings. Its just a word.

Either way, you should go talk to the website admins about z20. Its a good idea and sorely needed. Unless you were just saying it. Either way, I support it.

I will say though that the high level of passive aggressiveness people have toward each other here gets pretty grating sometimes. Everyone here is pretty polite. Its easy to be polite, but its not all there is. It takes more then that to be thoughtful and respectful toward people. Thats just my opinion. Your allowed to disagree.

@matkoniecz
Copy link
Contributor

matkoniecz commented Dec 27, 2018

I dont mean trolling in a derogatory sense

This word as used in English language, especially online, has solely strongly derogatory meaning.

@Adamant36
Copy link
Contributor

@matkoniecz, there's also how the person intends it and I didn't intend it in a derogatory manor. I've never used the word before and I didn't put that much thought into it. As I said though, it was still my bad and I wont use it again. There is that whole thing about interpreting the arguments of others in good faith in the code of conduct, but why should that apply when it comes to the usage of words like "troll" right?

I know language policing is an extremely easy way to critique someones feedback and undermined their argument. So its on me to make sure I'm 100% solid on the words I use. So the rest of what I say wont be discounted by people who use low hanging fruit like that as a rhetorical method.

For the sake of fairness though if your going to cite the rules of conduct to me for the usage of a word, have you should also cite them to @sommerluk for going off topic by bringing up a subject that is not constructive to the current issue and has already been criticized multiple times, by multiple people including @sommerluk. Its not like those things can't be discussed in the road map or governing status issues if need be. I'm sick of hearing about it everywhere else though. I think they are valid opinions, their just separate issues from this and the other ones where it keeps getting brought up.

@matkoniecz
Copy link
Contributor

there's also how the person intends it

People are reading published text, not your mind. What you intended to write is not changing meaning of used words.

It is good to know that you were not planning to insult anybody, but used words were unfortunately unambiguously insulting.

@Tomasz-W Tomasz-W mentioned this pull request Jan 1, 2019
19 tasks
@kocio-pl
Copy link
Collaborator

kocio-pl commented Jan 3, 2019

It's hard to say what is the status of this ticket, since technically it looks simple, but there is unresolved disagreement about initial zoom levels (and I'm happy with z19+ code).

@matthijsmelissen What do you think?

@matthijsmelissen
Copy link
Collaborator

  • I'm against making rules for objects that are only visible on z20.
  • I checked some stations, and in the stations I checked, rendering ticket machines at z19 would not lead to too much clutter.
  • In for example a park, I can imaging people wanting to search for excrement bags. In addition, it might also help people to orient themselves in the park. Also for this purpose I think rendering at z19 is fine.

So I'm going to merge this PR.

@matthijsmelissen matthijsmelissen merged commit 8229232 into gravitystorm:master Jan 4, 2019
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.

None yet

6 participants