-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 support for 19 digit cc #1177
Conversation
added support for 19 digit visa and discover
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, thanks for your PR! 🎉
Could you make these changes:
- Other that change the current regex for 16-digit CC, can you create another one for 19-digit which can be switched to by checking the string length?
- Update the tests to cover for the 19-digit case
Hi @profnandaa do you want some kind of condition to check the length, and if its 19 characters do the new one, if not original one ? the original one is not just for 16 but also supports shorter ones. is your concern about performance or readability ? if its readability maybe breaking it out to match specific cases would be the best ? I have noticed that sometimes there are valid numbers which would not be valid cc number ( based on this https://www.freeformatter.com/credit-card-number-generator-validator.html ) |
@aemarcuss -- got it. Let it be then. Just add the tests and we should be good to go. |
added tests
@profnandaa added & passed the tests. |
@profnandaa You reckon this is good to go? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, sorry for my delayed check on this one. Thanks for your contrib! 🎉
LGTM.
added support for 19 digit visa and discover cc