Skip to content

Ie model fields#360

Closed
paulcunnane wants to merge 14 commits intodjango:masterfrom
paulcunnane:ie-model-fields
Closed

Ie model fields#360
paulcunnane wants to merge 14 commits intodjango:masterfrom
paulcunnane:ie-model-fields

Conversation

@paulcunnane
Copy link
Copy Markdown
Contributor

@paulcunnane paulcunnane commented Feb 20, 2019

This change adds an "EircodeField" to the Irish local flavour. An Eircode is
an Irish postcode (https://www.eircode.ie). The format of the Eircode is
validated, but its existence is not.

Thanks for your contribution!

A checklist is included below which helps us keep the code contributions
consistent and helps speed up the review process. You can add additional
commits to your pull request if you haven't met all of these points on your
first version.

All Changes

  • Add an entry to the docs/changelog.rst describing the change.

  • Add an entry for your name in the docs/authors.rst file if it's not
    already there.

  • Adjust your imports to a standard form by running this command:

    `isort --recursive --line-width 120 localflavor tests`
    

New Fields Only

  • Prefix the country code to all fields.

    I haven't done this, because "Eircode" is a uniquely Irish name for
    a postcode.
    
  • Field names should be easily understood by developers from the target
    localflavor country. This means that English translations are usually
    not the best name unless it's for something standard like postal code,
    tax / VAT ID etc.

  • Prefer 'PostalCodeField' for postal codes as it's
    international English; ZipCode is a term specific to the United
    States postal system.

    Again, it's called "Eircode" because we just don't call them postal codes.
    
  • Add meaningful tests. 100% test coverage is not required but all
    validation edge cases should be covered.

  • Add documentation for all fields.

@benkonrath
Copy link
Copy Markdown
Member

Thanks for your contribution. Can you rebase this branch with master? That will pickup the latest changes and fixes to the CI config and the tests should pass. I'll do the review once you've rebased. Thanks.

@paulcunnane
Copy link
Copy Markdown
Contributor Author

I think that's done now (not a git expert!)

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 25, 2019

Codecov Report

Merging #360 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   95.93%   95.94%   +0.01%     
==========================================
  Files         157      157              
  Lines        3981     3996      +15     
  Branches      523      525       +2     
==========================================
+ Hits         3819     3834      +15     
  Misses         98       98              
  Partials       64       64
Impacted Files Coverage Δ
localflavor/generic/validators.py 100% <ø> (ø) ⬆️
localflavor/ie/forms.py 100% <100%> (ø) ⬆️
localflavor/no/forms.py 87.35% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cb223b...c0d32d8. Read the comment docs.

@benkonrath benkonrath added this to the 2.2 milestone Apr 22, 2019
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 this pull request may close these issues.

3 participants