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.

Sign up for GitHub

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

Jump to bottom

Add local flavor for Ukraine. #273

Merged
merged 3 commits into from
Feb 7, 2017
Merged

Add local flavor for Ukraine. #273

merged 3 commits into from
Feb 7, 2017

Conversation

illia-v
Copy link
Contributor

@illia-v illia-v commented Dec 27, 2016

No description provided.

@illia-v illia-v force-pushed the Ukrainian-localflavor branch 2 times, most recently from 666e57c to b06bbfd Compare December 27, 2016 22:05
benkonrath
benkonrath requested changes Jan 12, 2017
View reviewed changes
Copy link
Member

@benkonrath benkonrath left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. It came a little late to get it into 1.4 but I'm hoping to do a 1.5 release in a month or two so you shouldn't have to wait too long to get this into a release version of localflavor.

localflavor/ua/forms.py
"""
A Select widget that uses a list of Ukrainian regions as its choices.

.. versionadded:: 1.4
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to target 1.5 for this PR. Please change all versionadded labels to 1.5.

localflavor/ua/ua_regions.py
#: 24 oblasts, Autonomous Republic of Crimea and 2 cities with special status
# Codes were gotten from KOATUU
UA_REGION_CHOICES = (
('71', _('Cherkasy Oblast')),
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that you use ISO_3166-2:UA for the stored code instead of KOATUU. According to the KOATUU English wikipedia page, KOATUU will be integrated into ISO_3166-2:UA at some point in the future. Using the ISO standard also simplifies the discussion around disputed territories because we can just follow the standard. Does using ISO_3166-2:UA make sense to you?

I should point out that not everybody will be happy with ISO_3166-2:UA in regards to Crimea and Sevastopol being part of Ukraine. The solution you've implemented should allow developers to monkey-patch a region list that is suitable for their target audience so I'm ok with this PR being merged with Crimea and Sevastopol in the list.

@benkonrath benkonrath added this to the 1.5 milestone Jan 12, 2017
@benkonrath benkonrath self-assigned this Jan 12, 2017
@illia-v illia-v force-pushed the Ukrainian-localflavor branch from 3c09cb6 to fbb975e Compare January 15, 2017 12:22
@illia-v
Copy link
Contributor Author

illia-v commented Jan 15, 2017

@benkonrath, thanks. I have changed what you requested.

@illia-v illia-v force-pushed the Ukrainian-localflavor branch from dc20049 to 000772a Compare January 15, 2017 12:33
@benkonrath
Copy link
Member

Thanks for switching to the ISO 3166-2:UA codes. I meant that only the codes should be changed, not the name of the regions. It's better to use the English version of the region names instead of the transliterated region names because English is the base language for translations. You can find the English names on this page:
https://en.wikipedia.org/wiki/Administrative_divisions_of_Ukraine#Regions

Sorry for the confusion. Does this make sense to you?

@illia-v
Copy link
Contributor Author

illia-v commented Jan 29, 2017

@benkonrath, changed that. Sorry for delay.

@benkonrath benkonrath merged commit ecbcff4 into django:master Feb 7, 2017
@benkonrath
Copy link
Member

Thanks again for your contribution!

@illia-v illia-v deleted the Ukrainian-localflavor branch February 7, 2017 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benkonrath benkonrath benkonrath approved these changes

Assignees

@benkonrath benkonrath

Labels
enhancement
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.
2 participants
@illia-v @benkonrath