Opened 5 years ago

Closed 4 years ago

#17864 closed New feature (fixed)

Add localflavor for Hong Kong, starting with a phone number field

Reported by: mrkschan@… Owned by: KS Chan
Component: contrib.localflavor Version: 1.3
Severity: Normal Keywords: localflavor hk
Cc: Adrien Lemaire Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Hong Kong, ISO 3166 country code hk (http://is.gd/RIte7H), needs a local flavor for her phone number.

Attachments (4)

django.contrib.localflavor.hk.diff (3.2 KB) - added by KS Chan 5 years ago.
git-diff
hkphonenumber.2.diff (5.9 KB) - added by Adrien Lemaire 5 years ago.
Field fixed + Tests + translatable strings + Doc updated
hkphonenumber.diff (5.9 KB) - added by Adrien Lemaire 5 years ago.
Field fixed + Tests + translatable strings + Doc updated
hkphonenumber.3.diff (6.5 KB) - added by Adrien Lemaire 5 years ago.
Fixed with complete test + doc

Download all attachments as: .zip

Change History (16)

comment:1 Changed 5 years ago by KS Chan

Has patch: set
Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to KS Chan
Patch needs improvement: unset
Status: newassigned

comment:2 Changed 5 years ago by Aymeric Augustin

Easy pickings: set
Needs documentation: set
Needs tests: set
Patch needs improvement: set
Summary: Add Hong Kong phone number to django.contrib.localflavor.hkAdd localflavor for Hong Kong, starting with a phone number field
Triage Stage: UnreviewedAccepted

I've rephrased the title to make it clearer that this patch adds a new country to localflavor.

We generally don't include references to South in Django, that comment should be removed.

This needs tests and docs, but they should be very easy to write, using other countries as examples.

comment:3 in reply to:  2 Changed 5 years ago by KS Chan

Replying to aaugustin:

I've rephrased the title to make it clearer that this patch adds a new country to localflavor.

We generally don't include references to South in Django, that comment should be removed.

This needs tests and docs, but they should be very easy to write, using other countries as examples.

I find no test under django/contrib/localflavor/ in the source distribution tarball (http://www.djangoproject.com/download/1.3.1/tarball/), am I looking at the right place?

Last edited 5 years ago by KS Chan (previous) (diff)

comment:4 Changed 5 years ago by Aymeric Augustin

Changed 5 years ago by KS Chan

git-diff

comment:5 Changed 5 years ago by KS Chan

I didn't have experience in writing proper django style docstrings. Could you give me hints? See attached the new patch.

comment:6 Changed 5 years ago by Adrien Lemaire

mrkschan, look at https://docs.djangoproject.com/en/1.3/internals/documentation/ for django-specific markup

Can't see any test in the diff as well

All strings should be translatable, you need ugettext_lazy for the default_error_messages

You should remove your localflavor/hk/models.py file, that doesn't make sense. A 'PhoneNumberField' field already exists in localflavor/us/models.py

missing init.py in localflavor/hk/

forms.py should better starts with "from future import absolute_import"

missing doc

Last edited 5 years ago by Adrien Lemaire (previous) (diff)

comment:7 Changed 5 years ago by Adrien Lemaire

Needs tests: unset
Patch needs improvement: unset

Changed 5 years ago by Adrien Lemaire

Attachment: hkphonenumber.2.diff added

Field fixed + Tests + translatable strings + Doc updated

Changed 5 years ago by Adrien Lemaire

Attachment: hkphonenumber.diff added

Field fixed + Tests + translatable strings + Doc updated

comment:8 Changed 5 years ago by Adrien Lemaire

Cc: Adrien Lemaire added
Needs documentation: unset

comment:9 Changed 5 years ago by Julien Phalip

Triage Stage: AcceptedReady for checkin

Thanks!

comment:10 in reply to:  8 Changed 5 years ago by KS Chan

Replying to Fandekasp:

Missed a valid testcase -

'85291111111': '9111-1111',

Missed a invalid testcase -

'2111--1111': [error_msgs['invalid'], ]

i have prepared a set of comprehensive test cases - see http://is.gd/QE13Zs

Changed 5 years ago by Adrien Lemaire

Attachment: hkphonenumber.3.diff added

Fixed with complete test + doc

comment:11 Changed 5 years ago by Adrien Lemaire

Done, glad to be of help.

I also modified the invalid error message to inform about all acceptable inputs.

comment:12 Changed 4 years ago by Julien Phalip

Resolution: fixed
Status: assignedclosed

In [17886]:

Fixed #17864 -- Added Hong Kong localflavor. Thanks to mrkschan and Adrien Lemaire.

Note: See TracTickets for help on using tickets.
Back to Top