FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Feat: enchance isCreditCard by brianwhaley · Pull Request #2008 · validatorjs/validator.js · GitHub

/ validator.js Public

Feat: enchance isCreditCard - #2008

Merged
profnandaa merged 2 commits into
validatorjs:masterfrom
brianwhaley:feat/isAmexCard
Oct 17, 2022
ConversationCommits (2)ChecksFiles changed
Merged

Feat: enchance isCreditCard#2008
profnandaa merged 2 commits into
validatorjs:masterfrom
brianwhaley:feat/isAmexCard

Conversation

brianwhaley commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

Add regex validation for AmEx card

rubiin requested review from profnandaa and rubiin July 27, 2022 05:30
rubiin requested changes Jul 27, 2022
View reviewed changes

rubiin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

hey there, instead of making a case specific validator, I would suggest something like isDebitcard or isCreditCard followed by provider as a param

brianwhaley commented Jul 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

hey @rubiin i have refactored the isCreditCard file, will be submitting just submitted a new PR shortly. Thanks for you suggestions!

codecov Bot commented Jul 27, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #2008 (0eaded6) into master (1bb14e8) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2008   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          104       104           
  Lines         2203      2210    +7     
  Branches       477       481    +4     
=========================================
+ Hits          2203      2210    +7
Impacted Files Coverage Δ
src/lib/isCreditCard.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rubiin approved these changes Jul 28, 2022
View reviewed changes

rubiin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, lets wait for others

rubiin changed the title Feat/isAmexCard Feat/isCreditCard Jul 28, 2022

brianwhaley commented Jul 28, 2022

Copy link
Copy Markdown
Contributor Author

Thank you @rubiin. @profnandaa when you have an opportunity please do look at the PR. Much appreciated!

brianwhaley deleted the feat/isAmexCard branch July 28, 2022 13:18
brianwhaley restored the feat/isAmexCard branch July 28, 2022 15:50
brianwhaley reopened this Jul 28, 2022

brianwhaley commented Jul 28, 2022

Copy link
Copy Markdown
Contributor Author

apologies, thought this was approved and started cleaning up too soon. PR is reopened and waiting for approval from @profnandaa

rubiin self-requested a review July 28, 2022 17:39
rubiin added ready-to-land For PRs that are reviewed and ready to be landed and removed needs-more-review 🍿 discussion labels Jul 28, 2022
rubiin requested changes Jul 28, 2022
View reviewed changes

rubiin left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I found out that we already have isCreditCard. You can enhance it by adding your changes. We might have to consider releasing this on next major release as it would introduce a breaking change

rubiin changed the title Feat/isCreditCard Feat: enchance isCreditCard Jul 28, 2022

brianwhaley commented Jul 28, 2022

Copy link
Copy Markdown
Contributor Author

@rubiin - i did modify the existing isCreditCard file, and ensured it was backwards compatible. can you evaluate the diff? you will see you can still pass in a credit card number without the second param, and still get the same result. Thanks

rubiin previously approved these changes Jul 28, 2022
View reviewed changes

rubiin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

brianwhaley commented Jul 29, 2022

Copy link
Copy Markdown
Contributor Author

Hi @rubiin and @profnandaa - is there an additional person that can approve this PR? I would like to get it merged to main. Thank you!

rubiin commented Jul 29, 2022

Copy link
Copy Markdown
Member

Lets wait for @profnandaa

brianwhaley commented Aug 5, 2022

Copy link
Copy Markdown
Contributor Author

@profnandaa and @rubiin , is there an additional person that can approve this PR? I would like to get it merged to main. Thank you!

brianwhaley commented Aug 10, 2022

Copy link
Copy Markdown
Contributor Author

@WikiRik would you be able to review this PR ? Thank you!

WikiRik commented Aug 10, 2022

Copy link
Copy Markdown
Member

@brianwhaley I can, but my review won't get this merged faster unfortunately. We'll have to wait for @profnandaa

Either way you can still use this specific commit in other projects, see this NPM docs

florinMiro mentioned this pull request Jun 11, 2024
Closed
snyk-io Bot mentioned this pull request Jun 11, 2024
Closed
This was referenced Jun 12, 2024
Closed
Closed
snyk-io Bot mentioned this pull request Jun 13, 2024
Closed
DefenderK mentioned this pull request Jun 13, 2024
Closed
This was referenced Jun 14, 2024
Closed
Closed
DefenderK mentioned this pull request Jun 14, 2024
Closed
This was referenced Jun 20, 2024
Closed
Closed
snyk-io Bot mentioned this pull request Jun 28, 2024
Closed
This was referenced Jul 10, 2024
Closed
Closed
Closed
rm-neilsondesa mentioned this pull request Jul 16, 2024
Open
snyk-io Bot mentioned this pull request Jul 18, 2024
Closed
DefenderK mentioned this pull request Jul 18, 2024
Closed
This was referenced Jul 19, 2024
Closed
Closed
Closed
luizhp mentioned this pull request Aug 21, 2024
Open
This was referenced Aug 30, 2024
Open
Open
sumandas0 mentioned this pull request Sep 8, 2024
Open
TheJ-Erk400 mentioned this pull request Sep 8, 2024
Open
Bastien-Durand mentioned this pull request Sep 9, 2024
Open
KOTTAGENVH mentioned this pull request Sep 27, 2024
Open
DefenderK mentioned this pull request Oct 30, 2024
Closed
fossabot Bot mentioned this pull request Oct 28, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

profnandaa profnandaa approved these changes
rubiin rubiin left review comments

Assignees

No one assigned

Labels

🎉 first-pr ✅ LGTM ready-to-land For PRs that are reviewed and ready to be landed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL