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 icon for leisure=bird_hide #3340

Merged
merged 1 commit into from
Aug 14, 2018
Merged

Add icon for leisure=bird_hide #3340

merged 1 commit into from
Aug 14, 2018

Conversation

Adamant36
Copy link
Contributor

@Tomasz-W Tomasz-W mentioned this pull request Aug 12, 2018
26 tasks
@kocio-pl kocio-pl merged commit e9db8bc into gravitystorm:master Aug 14, 2018
@kocio-pl
Copy link
Collaborator

Thanks, works as expected.

@lakedistrictOSM
Copy link

Does this PR add rendering for the name of the bird hide too?

@kocio-pl
Copy link
Collaborator

Yes, it does:

apd1ufqw

@polarbearing
Copy link
Contributor

Where is the reference to birds in this icon?
It looks like man_made=binoculars, and does not indicate any wildlife.

@Adamant36
Copy link
Contributor Author

@polarbearing I might be wrong, but I seem to remember when you made the same comment in the original proposal that it went know where and more people were for it with just the binoculars. Personally I think the fact that it is a "bird hide" is a lot less important to the fact that's a wildlife lookout spot. Therefore binoculars work fine and can probably work in other similar situations also eventually. There doesn't need to be a specialized icon for every unique instance of animal watching in my opinion.

@polarbearing
Copy link
Contributor

Which or how many people exactly?

binoculars work fine and can probably work in other similar situations also

Then bird_hide.svg is a misnomer for the file.

@Adamant36
Copy link
Contributor Author

It's your complaint. I'm sure you can figure it out. Its not on me to prove your point for you, if you even have one.

Notice I said "eventually." Its not like the name can't be changed, eventually, when its used in other cases.

If you have that much of a problem with it, why not create a new issue for it or go comment in the old one so other people can be involved in the discussion? Or why didn't you voice your objection during the three days the pr sat here? I would of been happy to change it if there was reason to. That goes for all my PRs. Feel free to bring up any issues you have with them when they are waiting to be merged. Although, things should be mostly figured out in the original issue in my opinion.

@polarbearing
Copy link
Contributor

We are all volunteers not working on deadlines. I'm a bit surprised that an item that was undecided since April is rushed from PR into merge within less than 2 days. That does not leave enough time to comment.
As there was no formal vote, this is my interpretation of the discussion. You were not even participating:
pro binocular-only: lakedistrictOSM,
pro including animal : polarbearing, Tomasz-W, hjart, dieterdreist, turnsole80

@dieterdreist
Copy link

dieterdreist commented Aug 15, 2018 via email

@Adamant36
Copy link
Contributor Author

@polarbearing Tomasz-W Added it to his "Code needed to render prepared icons" list like a month ago. Know one commented on there that it wasn't resolved or should be removed. The one with binoculars was the only one where an .SVG was provided.

In a comment on April 15th both lakedistrictOSM and Kioco-pl said binoculars alone would work best.

Dktue reacted with a thumbs up to Tomasz-W post of the binoculars only icon.

Then a little after that you thumbs upped your own anti binoculars only comment along with Hjart, which is neither here nor there I guess, but a bit odd.

dieterdreist said "maybe also an eye could be used instead of the binoculars." Which seems more like spit balling then a definitive "I'm against this."

Tomasz-W later says "I think it's obvious that 'binoculars-only' version is more readable. I think leisure-green binoculars would be enough for this feature." That seems pretty not including an animal to me.

Right under that you once again thumbs upped your own comment of the binocular-bird icon, probably to make it look more evenly split then it was. Eigenwillig thumbs downed it. So that user can be added to the anti bird-binocular crowd.

Then Hjart said "Yes, a 'binoculars-only' certainly is more readable." Which seems to be more pro binoculars-only then not to me. Turnsole80 just said the goose icon looks good and that was before any of the other icons where proposed, including the binocular-only one. So you can't really say he was against it. Ultimately, readability matters and you where the only one against it being binoculars only.

Also, I don't have to participate in a discussion to write the code for it or do a PR on it, and I find it hilarious you thumbs upped your own comments.

@polarbearing
Copy link
Contributor

@Adamant36 - TL;DR
I did not criticise you for submitting a PR, that is fine and everybody's right. I said it was insufficiently discussed before merging.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 15, 2018

Well, that's probably directed at my work... 😄

I'm also volunteer, but I care for deadlines. It proved me to be the most effective single planning tool with such a fuzzy environment. I try to keep the flow, because if something is active, people tend to be more active too, which helps the project to be healthy teamwork. Sometimes I try to wait longer, when I'm not sure or when I expect controversies, but in this case I was not expecting it. I was ready to merge it immediately, but wanted to wait at least a day or two.

If more discussion is needed, I'm open for this. Nobody likes reverts, but it's also a tool one can use if the problem is too big and most of the time some update is enough.

With regard to viewing devices - I think that we can simply render them brown (like other amenities) if we ever care for them and even use a telescope metaphor if that's not enough. Also don't forget about the context - they will be most likely located in some touristic place, while bird hide is most probably located somewhere in the outdoor, like forest for example.

@Adamant36
Copy link
Contributor Author

@polarbearing OK. It seemed like you where saying I should of been involved in the original discussion in order to do the PR or to have an opinion about it, since you brought up my lack of participation in the issue. I might of just read it wrong though.

@polarbearing
Copy link
Contributor

I should of been involved in the original discussion in order to do the PR

No I did not say that, I said "submitting a PR ... is fine and everybody's right"

the context - they will be most likely located in some touristic place

One of the render examples above is at the coastline, so this could be seen as whale watching, ship spotting, or viewpoint.

A resolution could be to open up the code to include leisure=wildlife_hide to make clear it is more general than just about birds.

The wiki on these tags recommends combinations with man_made=tower - how does the tower icon interact with the watching icon?

@kocio-pl
Copy link
Collaborator

A resolution could be to open up the code to include leisure=wildlife_hide to make clear it is more general than just about birds.

I like this idea. Usage is low, but I like more systematic approach, not just counting case by case, and the wiki page is here:

https://wiki.openstreetmap.org/wiki/Tag%3Aleisure%3Dwildlife_hide

The wiki on these tags recommends combinations with man_made=tower - how does the tower icon interact with the watching icon?

I don't know what is preferred rendering, but tower prevails.

We have many more specific cases when we would like to define special tag priority, but this is general problem and I'm not sure there is sane way of tweaking SQL.

@Adamant36
Copy link
Contributor Author

I was actually planning on adding leisure=wildlife_hide in thr original PR for this reason, but decided not to because of the low numbers and lack of discussion about it in the original issue. I think it would be a good addition though despite that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new features Requests to render new features POI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants