Add credit card testing for strings. Closes #42#444
Add credit card testing for strings. Closes #42#444notjrbauer wants to merge 5 commits intohapijs:masterfrom
Conversation
lib/string.js
Outdated
There was a problem hiding this comment.
for loop is more readable
|
Nice. Just need to close the comments above. |
|
Is there a reason to use string() and not number() for this? |
|
Further, I think the implementation referenced in the original request #422 is nicer. |
|
btw, I don't mean to step on anyone's toes, so do accept the other implementation. I'm just adding mine because it's how I would have done it. |
|
I'm a little confused why #465 trumped this PR? A credit card number should be treated as a string, not a number for several reasons. You will never do math on a credit card number (JavaScript floating point math at that), and the implementation in #465 converts the number to a string internally. Additionally, this PR appears to have been approved, pending a few small tweaks. |
|
I think we can all agree it's fixed by #469. Reply if not. |
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
Tests taken from @Christopher-Bui's initial PR.