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

Presets for horseback riding and hiking routes #4912

Merged
merged 2 commits into from
Apr 9, 2018

Conversation

NopMap
Copy link
Contributor

@NopMap NopMap commented Mar 18, 2018

Added/extended some presets for travel on horseback
Extended hiking and horse route relations - elementary tags were missing

@NopMap
Copy link
Contributor Author

NopMap commented Mar 18, 2018

The failed checks seem to have nothing to do with the PR. All PRs for the last day are failing with the same two errors.

@NopMap
Copy link
Contributor Author

NopMap commented Apr 8, 2018

I wonder why there is no comment or any other response to this after three weeks.

@bhousel
Copy link
Member

bhousel commented Apr 9, 2018

I wonder why there is no comment or any other response to this after three weeks.

Sorry! There's a lot to go through and I've been doing other things. Also I mentally ignore PRs with the tests failing - it usually means the person is still working on it. (But I know in this case the failure was unrelated, so that's my fault for ignoring).

Just took a quick look, and I will probably merge most of it this week for our next release. I don't think we should add that horse drinking field to amenity=fountain, but other than that nothing really stands out as a problem.

@bhousel bhousel merged commit 06058c1 into openstreetmap:master Apr 9, 2018
@bhousel
Copy link
Member

bhousel commented Apr 9, 2018

Ok, I cleaned it up and merged.. Some things I changed:

  • Proper Case all the things
  • Remove the trailblazing_ fields, which require special knowledge to use
  • Fix the icon on trail_riding_station preset
  • Make the horse_ checkboxes more consistent (yes/no)
  • Remove watering_place field, remove it from amenity=fountain preset
  • Update build script to not include "undefined" values in taginfo.json

This was a really tricky one to review because no other preset in iD works like this Trail Riding Station.
For most features - there is one main tag, and the fields just add detail to it.

For this feature - the "main" tag is "tourism": "trail_riding_station" and it uses checkboxes to hang on additional amenity, sport, leisure tags. It's totally backwards from how every other feature in OSM works. This is why you needed to use "matchScore" to avoid having other presets get matched when you check the boxes. 😬

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