Skip to content

/pydantic

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

PaymentCardNumber type #790

Merged
merged 31 commits into from Sep 17, 2019
Merged

PaymentCardNumber type #790

merged 31 commits into from Sep 17, 2019

Conversation

@matin
Copy link
Contributor

matin commented Sep 1, 2019 •
edited

Change Summary

Validation of payment card numbers.

Related issue number

closes #788

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.rst file added describing change
    (see changes/README.md for details)
matin added 7 commits Aug 31, 2019
@matin
initial proposal of payment card validation #788
7837567
@matin
some cleanup for PaymentCardNumber #788
1fd0f97
@matin
fix linting issues
09476a0
@matin
tests and more robust errors #788
Loading status checks…
3540786
@matin
fixed linting
Loading status checks…
8debdbf
@matin
fixing mypy errors
Loading status checks…
5600955
@matin
let's try this to make mypy work
Loading status checks…
43bb78c
@matin

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

matin commented Sep 1, 2019

@samuelcolvin How does the code look to you?
@matin
matin reviewed Sep 1, 2019
View changes
pydantic/types.py Outdated Show resolved Hide resolved
@matin
make consistent with cpython tests
Loading status checks…
979d306
@codecov

This comment has been minimized.

Sign in to view
Copy link

codecov bot commented Sep 1, 2019 •
edited

Codecov Report

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

@@          Coverage Diff          @@
##           master   #790   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          17     17           
  Lines        2889   2972   +83     
  Branches      568    578   +10     
=====================================
+ Hits         2889   2972   +83
Impacted Files Coverage Δ
pydantic/types.py 100% <100%> (ø) ⬆️
pydantic/errors.py 100% <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 f08fd2f...c667047. Read the comment docs.
@dmontagu
dmontagu reviewed Sep 1, 2019
View changes
pydantic/types.py Outdated Show resolved Hide resolved
@matin
avoid using __new__
Loading status checks…
eb3bb2a
@dmontagu
dmontagu reviewed Sep 1, 2019
View changes
pydantic/types.py Outdated Show resolved Hide resolved
matin added 4 commits Sep 1, 2019
@matin
use __init__ instead of properties
Loading status checks…
d139702
@matin
clean up PaymentCardNumber.__init__
Loading status checks…
6f8cdba
@matin
use ClassVar where applicable
Loading status checks…
aab2470
@matin
add to changes/
Loading status checks…
5da0f5b
@matin

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

matin commented Sep 1, 2019

@dmontagu thanks for the help!
matin added 3 commits Sep 1, 2019
@matin
turns out this is never used
888a070
@matin
get brand for other
Loading status checks…
3642448
@matin
clean up a bit
Loading status checks…
7765fae
@samuelcolvin
samuelcolvin reviewed Sep 2, 2019
View changes
Copy link
Owner

samuelcolvin left a comment

In general looks good, a few things to fix and please add docs.
changes/790-matin.rst Outdated
@@ -0,0 +1 @@
add the `PaymentCardNumber` type

This comment has been minimized.

Sign in to view
Copy link
@samuelcolvin

samuelcolvin Sep 2, 2019

Owner
Suggested change
add the `PaymentCardNumber` type
add the ``PaymentCardNumber`` type

This is rst :-)

This comment has been minimized.

Sign in to view
Copy link
@matin

matin Sep 2, 2019

Author Contributor

will do!

This comment has been minimized.

Sign in to view
Copy link
@samuelcolvin

samuelcolvin Sep 15, 2019

Owner

This still needs changing.

This comment has been minimized.

Sign in to view
Copy link
@samuelcolvin

samuelcolvin Sep 16, 2019

Owner

Hi @matin this still neds to change.
pydantic/types.py Show resolved Hide resolved
pydantic/types.py Show resolved Hide resolved
pydantic/types.py Outdated Show resolved Hide resolved
@samuelcolvin
samuelcolvin reviewed Sep 3, 2019
View changes
Copy link
Owner

samuelcolvin left a comment

Just a few more small things, sorry to be a pedant.
pydantic/errors.py Outdated Show resolved Hide resolved
pydantic/errors.py Outdated Show resolved Hide resolved
pydantic/types.py Outdated Show resolved Hide resolved
tests/test_types_payment_card_number.py Outdated Show resolved Hide resolved
tests/test_types_payment_card_number.py Outdated Show resolved Hide resolved
tests/test_types_payment_card_number.py Outdated Show resolved Hide resolved
tests/test_types_payment_card_number.py Outdated Show resolved Hide resolved
tests/test_types_payment_card_number.py Outdated Show resolved Hide resolved
@samuelcolvin

This comment has been minimized.

Sign in to view
Copy link
Owner

samuelcolvin commented Sep 3, 2019

oh, and conflicts.
matin added 5 commits Sep 13, 2019
@matin
Merge branch 'master' into payment_card
a0b07f4
@matin
make PaymentCardNumber private
ab30db7
@matin
use if/else to make faster in cython
819fb29
@matin
card_number => card number in errors
981d82c
@matin
make it clearer in the tests which exception is being raised
64dbe23
@matin
parameterize tests for PaymentCardNumber
Loading status checks…
83329cf
@matin matin force-pushed the cuenca-mx:payment_card branch from 8bc2510 to 83329cf Sep 14, 2019
@matin
lint
Loading status checks…
6adc007
@samuelcolvin
samuelcolvin reviewed Sep 15, 2019
View changes
changes/790-matin.rst Outdated
@@ -0,0 +1 @@
add the `PaymentCardNumber` type

This comment has been minimized.

Sign in to view
Copy link
@samuelcolvin

samuelcolvin Sep 15, 2019

Owner

This still needs changing.
pydantic/types.py Outdated Show resolved Hide resolved
pydantic/types.py Outdated Show resolved Hide resolved
matin added 5 commits Sep 15, 2019
@matin
clean up merge mistake from earlier
Loading status checks…
a2fe27a
@matin
fix mypy test error
Loading status checks…
bb6a2a4
@matin
take out commented line
Loading status checks…
8339c93
@matin
update comment since code is moved
Loading status checks…
be3ba41
@matin
docs for PaymentCardNumber
Loading status checks…
56cbe68
@samuelcolvin
samuelcolvin reviewed Sep 16, 2019
View changes
docs/examples/payment_card_number.py Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
matin added 3 commits Sep 16, 2019
@matin
Merge branch 'master' into payment_card
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7436617
@matin
clean up docs for PaymentCardNumber
Loading status checks…
b3752f9
@matin
var name that makes more sense
Loading status checks…
c667047
@samuelcolvin samuelcolvin merged commit 0c57937 into samuelcolvin:master Sep 17, 2019
10 checks passed
10 checks passed
Header rules No header rules processed
Details
Pages changed All files already uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
codecov/project 100% (+0%) compared to f08fd2f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
samuelcolvin.pydantic Build #20190916.24 succeeded
Details
samuelcolvin.pydantic (Job Python36) Job Python36 succeeded
Details
samuelcolvin.pydantic (Job Python37) Job Python37 succeeded
Details
@samuelcolvin

This comment has been minimized.

Sign in to view
Copy link
Owner

samuelcolvin commented Sep 17, 2019

Awesome. Thank you.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelcolvin samuelcolvin

@dmontagu dmontagu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@matin @samuelcolvin @dmontagu
You can’t perform that action at this time.