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

added areas in Kuwait #296

Closed
wants to merge 12 commits into from
Closed

added areas in Kuwait #296

wants to merge 12 commits into from

Conversation

Dreamersoul
Copy link
Contributor

@Dreamersoul Dreamersoul commented Jun 15, 2017

Please replace these instructions with a description of your change. The
'New Fields Only' section should be removed if your pull request
doesn't add any new fields.

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.

  • 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.

  • Add meaningful tests. 100% test coverage is not required but all
    validation edge cases should be covered.

  • Add .. versionadded:: <next-version> comment markers to new
    localflavors.

  • Add documentation for all fields.

@Dreamersoul
Copy link
Contributor Author

i dont know why but the isort --recursive --line-width 120 localflavor tests changed all the test files for some reason

sorry if i missed something this is the first time i contribute here please let me know if something is missing

@Dreamersoul
Copy link
Contributor Author

are there any core contributors that can help me with this PR? i dont know why it is failing

@claudep
Copy link
Member

claudep commented Jun 19, 2017

Hey Hamad,
Not related to the failure, but you should first revert all the blank line removal in your second commit. These unneeded changes make your pull request more difficult to review.
Then, follow the advice in the failure: set self.maxDiff = None in your test case, and you might then see where is the output difference.

@codecov-io
Copy link

codecov-io commented Jun 20, 2017

Codecov Report

Merging #296 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #296      +/-   ##
=========================================
+ Coverage   96.29%   96.3%   +<.01%     
=========================================
  Files         151     152       +1     
  Lines        4214    4221       +7     
  Branches      579     579              
=========================================
+ Hits         4058    4065       +7     
  Misses         97      97              
  Partials       59      59
Impacted Files Coverage Δ
localflavor/kw/kw_areas.py 100% <100%> (ø)
localflavor/kw/forms.py 89.58% <100%> (+0.94%) ⬆️

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 961fd7d...8cdaa74. Read the comment docs.

@Dreamersoul
Copy link
Contributor Author

Thank you @claudep I have reverted what isort did to the files not related to my commit and the tests pass now!
However I don't know what to do to satisfy the following requirement:

Add .. versionadded:: <next-version> comment markers to new
localflavors.

@claudep
Copy link
Member

claudep commented Jun 20, 2017

You can grep the code for versionadded to see what it looks like.
See some docstrings of https://github.com/django/django-localflavor/blob/master/localflavor/au/validators.py for example.

@Dreamersoul
Copy link
Contributor Author

@claudep alright done 👍 thanks for your help 😄

@benkonrath
Copy link
Member

Merged via e51e52f.

Thanks!

@benkonrath benkonrath closed this Jun 30, 2017
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