Opened 12 years ago

Closed 12 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 12 years ago.
git-diff
hkphonenumber.2.diff (5.9 KB ) - added by Adrien Lemaire 12 years ago.
Field fixed + Tests + translatable strings + Doc updated
hkphonenumber.diff (5.9 KB ) - added by Adrien Lemaire 12 years ago.
Field fixed + Tests + translatable strings + Doc updated
hkphonenumber.3.diff (6.5 KB ) - added by Adrien Lemaire 12 years ago.
Fixed with complete test + doc

Download all attachments as: .zip

Change History (16)

comment:1 by KS Chan, 12 years ago

Has patch: set
Owner: changed from nobody to KS Chan
Status: newassigned

comment:2 by Aymeric Augustin, 12 years ago

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.

in reply to:  2 comment:3 by KS Chan, 12 years ago

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 12 years ago by KS Chan (previous) (diff)

comment:4 by Aymeric Augustin, 12 years ago

by KS Chan, 12 years ago

git-diff

comment:5 by KS Chan, 12 years ago

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

comment:6 by Adrien Lemaire, 12 years ago

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 12 years ago by Adrien Lemaire (previous) (diff)

comment:7 by Adrien Lemaire, 12 years ago

Needs tests: unset
Patch needs improvement: unset

by Adrien Lemaire, 12 years ago

Attachment: hkphonenumber.2.diff added

Field fixed + Tests + translatable strings + Doc updated

by Adrien Lemaire, 12 years ago

Attachment: hkphonenumber.diff added

Field fixed + Tests + translatable strings + Doc updated

comment:8 by Adrien Lemaire, 12 years ago

Cc: Adrien Lemaire added
Needs documentation: unset

comment:9 by Julien Phalip, 12 years ago

Triage Stage: AcceptedReady for checkin

Thanks!

in reply to:  8 comment:10 by KS Chan, 12 years ago

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

by Adrien Lemaire, 12 years ago

Attachment: hkphonenumber.3.diff added

Fixed with complete test + doc

comment:11 by Adrien Lemaire, 12 years ago

Done, glad to be of help.

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

comment:12 by Julien Phalip, 12 years ago

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