-
Notifications
You must be signed in to change notification settings - Fork 217
Cleanup presets and fields to adhere to stricter schema #1759
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
Conversation
|
🍱 Your pull request preview is ready Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone. FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes. |
| "ref", | ||
| "direction_vertex" | ||
| ], | ||
| "moreFields": [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it (or elsewhere) changing behaviour by blocking implicit inheritance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked this one and a few others (with both fields and moreFields) and they previously didn't block implicit inheritance (which also matches the implementation: https://github.com/openstreetmap/iD/blob/d9b070be735d76e6c3d47bf4d532cde42d9d0e8c/modules/presets/preset.js#L295-L336) and don't do it either now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then it is quite useful as I would expect empty array to block it
matkoniecz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annoying to test, but seems to not break anything?
and clarifying implicit inheritance status is helpful
|
In |
|
Oh, thanks for noticing and reverting! Sorry I didn't catch this during testing. |
Description, Motivation & Context
Fix issues found by ideditor/schema-builder#242:
keysfrom fieldstermsfrom presetsaddTags/removeTagsfrom presetsfields/moreFieldsfrom presetstermsfrom presetsRelated issues
Links and data
Relevant OSM Wiki links: –
Relevant tag usage stats: –
Checklist and Test-Documentation Template
Read on to get your PR merged faster…
Follow these steps to test your PR yourself and make it a lot easier and faster for maintainers to check and approve it.
This is how it works:
After you submit your PR, the system will create a preview and comment on your PR:
Once the preview is ready, use it to test your changes.
Now copy the snippet below into a new comment and fill out the blanks.
Now your PR is ready to be reviewed.