Skip to content

UK specific Post Box tagging.#804

Merged
tyrasd merged 9 commits intoopenstreetmap:mainfrom
UKChris-osm:post-box
Mar 1, 2023
Merged

UK specific Post Box tagging.#804
tyrasd merged 9 commits intoopenstreetmap:mainfrom
UKChris-osm:post-box

Conversation

@UKChris-osm
Copy link
Copy Markdown
Contributor

@UKChris-osm UKChris-osm commented Mar 1, 2023 •
edited
Loading

#670 @boothym

Added location exclusion to the current post box file, excluding gb.
Added new file for a gb post box to promote gb specific tagging.

@tyrasd tyrasd added regional new-field create a new field (see add-field for cases where field from presets is added to new entries) labels Mar 1, 2023
@UKChris-osm
Copy link
Copy Markdown
Contributor Author

UKChris-osm commented Mar 1, 2023

I took post_box:type from the the wiki, but I see it's causing a build error, is that an error on my part?

tyrasd
tyrasd requested changes Mar 1, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@tyrasd tyrasd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The definition of the two new fields (post_box:type and royal_cypher) are still missing in this PR.

They should probably both by of "type": "combo", I assume.

Comment thread data/presets/amenity/post_box/post_box_gb.json Outdated
Comment thread data/presets/amenity/post_box/post_box_gb.json Outdated
@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Mar 1, 2023

PS: The missing fields should go into data/fields, see https://github.com/openstreetmap/id-tagging-schema/blob/main/data/fields/piste/type.json for an example of a similar combo field.

PSS: I like that you put the regional preset into a new subdirectory (data/presets/amenity/post_box/). But could you please rename the file to post_box-GB.json, following the naming convention we use for regional presets.

@UKChris-osm
Copy link
Copy Markdown
Contributor Author

UKChris-osm commented Mar 1, 2023

I've tried applying the suggested changes, hopefully I have done so accurately.

@UKChris-osm
Copy link
Copy Markdown
Contributor Author

UKChris-osm commented Mar 1, 2023

Thanks for helping me through this @tyrasd, sorry for my syntax errors.

@tyrasd tyrasd merged commit 5014680 into openstreetmap:main Mar 1, 2023
@UKChris-osm UKChris-osm deleted the post-box branch March 1, 2023 21:31
@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Mar 1, 2023

No worries. Thanks for your first PR to the tagging schema! 😃

@UKChris-osm
Copy link
Copy Markdown
Contributor Author

UKChris-osm commented Mar 1, 2023

How long until this tagging will be usable within iD?

closes #670

@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Mar 1, 2023

Not long. I've been planning to do a proper release for 6.0 soon anyway.

@UKChris-osm UKChris-osm mentioned this pull request Mar 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tyrasd tyrasd tyrasd approved these changes

+1 more reviewer

@boothym boothym boothym left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

new-field create a new field (see add-field for cases where field from presets is added to new entries) regional

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@UKChris-osm @tyrasd @boothym
⚡