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

Update marina.json #7622

Closed
wants to merge 2 commits into from
Closed

Update marina.json #7622

wants to merge 2 commits into from

Conversation

Eric-Sparks
Copy link
Contributor

Moved some fields from 'moreFields' to 'Fields' to get better visability into what information is important to document with marinas.
Also added a few tags that will help capture information that is helpful.
Added proper seamark tags.

Moved some fields from 'moreFields' to 'Fields' to get better visability into what information is important to document with marinas.
Also added a few tags that will help capture information that is helpful.
Added proper seamark tags.
@Eric-Sparks
Copy link
Contributor Author

I'm not exactly sure how to propose such a change, but the sanitary_dump_station for boats is different from RVs. Simply marking it as "yes" doesn't really map it correctly and could lead to some issues with RVers navigating to the wrong location.

Actually, I think I just found the proper tag... waterway=sanitary_dump_station instead of using amenity=sanitary_dump_station.

Copy link
Collaborator

@quincylvania quincylvania left a comment

Choose a reason for hiding this comment

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

Looks like there are a few issues here. You can review the presets readme to learn more about the file format. Also, be sure to run npm run build:data to check for errors and include the derived data changes as well.

Comment on lines +17 to +20
"maxdraft",
"maxairdraft",
"power_supply",
"vhf_channel"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think all these fields exist yet. You'll have to add them in files under data/presets/fields.

Comment on lines +39 to +40
"seamark:type": "harbour",
"seamark:harbour:category": "marina"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should go under addTags since otherwise this preset won't match any existing features that don't have them.

@@ -3,24 +3,28 @@
"fields": [
"name",
"operator",
"capacity",
"phone",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We try to limit the fields array to only a few of the most important fields since some mappers feel like they have to fill out everything. Lots of complex default fields can become intimidating. Please keep the advanced fields under moreFields.

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