-
Notifications
You must be signed in to change notification settings - Fork 900
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
Added some Israel chains #10499
base: main
Are you sure you want to change the base?
Added some Israel chains #10499
Conversation
@@ -10670,6 +10670,7 @@ | |||
"displayName": "יינות ביתן", | |||
"id": "yeinotbitan-6b65f1", | |||
"locationSet": {"include": ["il"]}, | |||
"matchNames": ["yaynot bitan"], | |||
"tags": { | |||
"alt_name:en": "Yenot Bitan; Bitan Wines", |
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.
Please remove space after semicolon
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.
hi, thanks for review!
Where is this space? I don't see it
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.
The space following the semicolon is in the value for alt_name:en
. Semicolons separating values of keys should not have a space following them per OSM conventions.
"id": "foxhome-a5093f", | ||
"locationSet": {"include": ["il"]}, | ||
"id": "foxhome-43585e", | ||
"locationSet": {"include": ["ca", "il"]}, |
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.
The Canadian stores should probably have a separate entry in the NSI that doesn't include the multilingual name tags. Also, the specified Wikidata item currently does not have any statements related to stores outside of Israel.
"displayName": "Laline", | ||
"id": "laline-255338", | ||
"locationSet": { | ||
"include": ["ca", "ge", "il", "jp", "tw"] |
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.
This locationSet
is highly problematic considering that each of the five countries listed not only have different primary languages, but different language scripts. If the brand does indeed operate in these countries, each country should probably have its own localized entry in the NSI.
Hi
Added some more Israeli chains and aligned some existing ones