Opened 13 years ago
Closed 13 years ago
#17864 closed New feature (fixed)
Add localflavor for Hong Kong, starting with a phone number field
| Reported by: | 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)
Change History (16)
comment:1 by , 13 years ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
follow-up: 3 comment:2 by , 13 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.hk → Add localflavor for Hong Kong, starting with a phone number field |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 13 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?
comment:4 by , 13 years ago
To work on Django, you should checkout the source distribution.
Tests are in tests.regressiontests.localflavor.
comment:5 by , 13 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 , 13 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 can already be found in localflavor/us/models.py
comment:7 by , 13 years ago
| Needs tests: | unset |
|---|---|
| Patch needs improvement: | unset |
by , 13 years ago
| Attachment: | hkphonenumber.2.diff added |
|---|
Field fixed + Tests + translatable strings + Doc updated
by , 13 years ago
| Attachment: | hkphonenumber.diff added |
|---|
Field fixed + Tests + translatable strings + Doc updated
follow-up: 10 comment:8 by , 13 years ago
| Cc: | added |
|---|---|
| Needs documentation: | unset |
comment:10 by , 13 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
comment:11 by , 13 years ago
Done, glad to be of help.
I also modified the invalid error message to inform about all acceptable inputs.
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.