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

Bug: Missing ISO 3166-2:SH (Saint Helena, Ascension and Tristan da Cunha) #255

Closed
blumk opened this issue Jan 6, 2018 · 11 comments · Fixed by #268
Closed

Bug: Missing ISO 3166-2:SH (Saint Helena, Ascension and Tristan da Cunha) #255

blumk opened this issue Jan 6, 2018 · 11 comments · Fixed by #268
Assignees

Comments

@blumk
Copy link
Contributor

blumk commented Jan 6, 2018

ISO 3166-2:SH is missing from the list.

I'm aware that these islands are not independent countries, nevertheless they are treated as a dependent country according to ISO-3166-1.

I've created an issue #252 (include 'independent' flag) that would help filtering the data set.

I recommend keeping this data set compliant with ISO-3166-1.

@mledoze
Copy link
Owner

mledoze commented Jan 21, 2018

@blumk the data is not missing, it's just in a different place :)

Please see this issue for more info: #93

@blumk
Copy link
Contributor Author

blumk commented Jan 21, 2018

@mledoze see my comment on #254. I believe aligning the dataset with ISO 3166 is the better solution.

Quoting ISO:

When the world agrees

I personally might not agree with every entry in the ISO standard, I do however believe in a standard that everyone agrees to (and then override the standard at my own risk in my application when needed).

3166:SH
Wikipedia

@blumk
Copy link
Contributor Author

blumk commented Jan 22, 2018

@mledoze I took a closer look at this issue:

TL;DR Why is this a bug?
If you want to use this data set for a drop down menu (e.g. shipping address) you'd expect 249 records in the list (ISO 3166), otherwise some clients will complain.

The independent flag allows user display the 194 countries if they do not need the extra details.

Details
The parent record 3166:SH should be included in the contries.json as it is a British Overseas Territory (like any other territory).

The record contains three islands (parts) and each of them should have a divisions record in the data folder. Since almost every country has subdivisions (states, departements, regions, ...) I'd say this is out of scope for this project, see ISO 3166-2. (There are over 4000 subdivisions in total)

Wikipedia

@mledoze
Copy link
Owner

mledoze commented Jan 23, 2018

@blumk I am not so sure about what you are proposing?

@blumk
Copy link
Contributor Author

blumk commented Jan 23, 2018

@mledoze I'd like to add ISO 3166-2:SH (Saint Helena, Ascension and Tristan da Cunha) to countries.json.

ISO 3166 lists 249 countries - the countries.json is just missing two records.

We can keep the division json files in data in case anyone is interested in a specific part of a country (or for legacy reasons).

@mledoze
Copy link
Owner

mledoze commented Jan 23, 2018

Now that we have the independent flag, it makes sense to move the files data/bes.divisions.json and data/shn.divisions.json back into the main countries.json file. We'll also remove the *.divisions.json files because they are not useful anymore.

We'll fix the issue #254 in the same time.

Do you think you could do that?

@blumk
Copy link
Contributor Author

blumk commented Jan 23, 2018

Yes, I was waiting for the major PRs to be completed first. It might take me a week to submit a PR for both issues.

@mledoze
Copy link
Owner

mledoze commented Jan 23, 2018

Ok. Do you prefer me to fix these issues?

I'll merge the PR regarding the arabic translations this week.

@blumk
Copy link
Contributor Author

blumk commented Jan 23, 2018

@mledoze You decide. I'm quite busy at work but willing to fix them if you need help. As soon as the arabic translations are in master it would be a good time for a merge.

I think the records have to be updated with the latest changes in countries.json (emoji support, languages and so on).

@mledoze
Copy link
Owner

mledoze commented Jan 23, 2018

I'm quite busy at work

Same here :)

I propose you fix these issues while I handle the merge of one of the two pull requests for arabic translations (there are still some problems with them).

I think the records have to be updated with the latest changes in countries.json (emoji support, languages and so on).

Of course, you work will not be lost!

By the way, thank you very much for your help on this project, it's much appreciated.

@blumk
Copy link
Contributor Author

blumk commented Jan 23, 2018

@mledoze Thanks for maintaining this project! I'll provide a fix for the two issues.

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 a pull request may close this issue.

2 participants