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.

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

Add 34 prefix value (ar localflavor ARCUITField). #342

Merged
merged 3 commits into from
Aug 2, 2018

Conversation

ramiro
Copy link
Member

@ramiro ramiro commented Jul 29, 2018

There are now CUIT assigned that use a 34 prefix. This PR adds support for it.

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`
    

@@ -11,7 +11,8 @@


class ARProvinceSelect(Select):
"""A Select widget that uses a list of Argentinean provinces/autonomous cities as its choices."""
"""A Select widget that uses a list of Argentinean provinces/autonomous
cities as its choices."""
Copy link
Member

Choose a reason for hiding this comment

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

I think that we generally go to next line after """ when it takes more than one line, but that's nit-picking...

Copy link
Member

Choose a reason for hiding this comment

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

I agree. We're using 120 character lines so you can also just not include this change. Either way is fine.

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! Nice to see that you added a test for this change. And thanks for correcting the small typos you found.

If you could make the 2 small updates, I will merge this. It would be nice to get this into the 2.1 release that I'm planning to do in a couple of weeks.

I can also make the changes if that's easier. Just let me know.

@@ -11,7 +11,8 @@


class ARProvinceSelect(Select):
"""A Select widget that uses a list of Argentinean provinces/autonomous cities as its choices."""
"""A Select widget that uses a list of Argentinean provinces/autonomous
cities as its choices."""
Copy link
Member

Choose a reason for hiding this comment

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

I agree. We're using 120 character lines so you can also just not include this change. Either way is fine.


More info:
http://es.wikipedia.org/wiki/Clave_%C3%9Anica_de_Identificaci%C3%B3n_Tributaria

English info:
Info in English:
http://www.justlanded.com/english/Argentina/Argentina-Guide/Visas-Permits/Other-Legal-Documents
"""
Copy link
Member

Choose a reason for hiding this comment

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

It would be great if you could add .. versionchanged:: 2.1 here.

@ramiro
Copy link
Member Author

ramiro commented Aug 2, 2018

Thank you for your reviews! I think I've taken care of them.

@codecov-io
Copy link

codecov-io commented Aug 2, 2018

Codecov Report

Merging #342 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   95.82%   95.82%           
=======================================
  Files         153      153           
  Lines        3878     3878           
  Branches      517      517           
=======================================
  Hits         3716     3716           
  Misses         98       98           
  Partials       64       64
Impacted Files Coverage Δ
localflavor/ar/forms.py 92.85% <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 0528736...f3a5b07. Read the comment docs.

@benkonrath benkonrath merged commit b5b0570 into django:master Aug 2, 2018
@ramiro ramiro deleted the add-34-cuilt-prefix-ar-lf branch August 7, 2018 14:30
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.

4 participants