Skip to content

feat(isEAN) - added support for EAN-14 along with test case, feat(isAlphanumeric)- added options(optional parameter)#1577

Merged
profnandaa merged 4 commits intovalidatorjs:masterfrom
varsubham:master
Mar 3, 2021
Merged

feat(isEAN) - added support for EAN-14 along with test case, feat(isAlphanumeric)- added options(optional parameter)#1577
profnandaa merged 4 commits intovalidatorjs:masterfrom
varsubham:master

Conversation

@varsubham
Copy link
Contributor

@varsubham varsubham commented Jan 15, 2021 •
edited
Loading

Fix Issue #1576
Fix Issue #1572

Checklist

@codecov
Copy link

codecov bot commented Jan 15, 2021 •
edited
Loading

Codecov Report

Merging #1577 (ff175d8) into master (1b85829) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1577   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          100       100           
  Lines         1796      1807   +11     
=========================================
+ Hits          1796      1807   +11
Impacted Files Coverage Δ
src/lib/isPassportNumber.js 100.00% <ø> (ø)
src/lib/isAlphanumeric.js 100.00% <100.00%> (ø)
src/lib/isDataURI.js 100.00% <100.00%> (ø)
src/lib/isEAN.js 100.00% <100.00%> (ø)
src/lib/isIdentityCard.js 100.00% <100.00%> (ø)
src/lib/isMACAddress.js 100.00% <100.00%> (ø)
src/lib/isStrongPassword.js 100.00% <100.00%> (ø)

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 1b85829...a300bab. Read the comment docs.

@varsubham varsubham changed the title added support for EAN-14 along with test case feat(isEAN) - added support for EAN-14 along with test case, feat(isAlphanumeric)- added options(optional parameter) Jan 18, 2021
@varsubham
Copy link
Contributor Author

varsubham commented Feb 5, 2021

@profnandaa Hey can you please review the PR its one of my first PR 😄.
If there are any errors please let me know. 🙂

@renanmontebelo
Copy link
Contributor

renanmontebelo commented Feb 6, 2021

I believe they are waiting for #1553 before reviewing any other PR.

@renanmontebelo
Copy link
Contributor

renanmontebelo commented Feb 6, 2021

I believe the PR should only include files under src/, test/ and README.md; /validator.js and /validator.min.js are automatically generated by the build process (and by npm test) if I'm not mistaken.

@varsubham
Copy link
Contributor Author

varsubham commented Feb 6, 2021

@renanmontebelo I thought that we need to check if all the test cases are running (npm test) before submitting a PR 🤔

@renanmontebelo
Copy link
Contributor

renanmontebelo commented Feb 6, 2021

These files are listed in .gitignore so I believe they shouldn't be added. Still, someone accidently added them that's why it's yielding as a modified file. See #1590 discussion.

tux-tn
tux-tn approved these changes Mar 1, 2021
View reviewed changes
Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

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

Thank you and congrats for your first contrib @varsubham . I deleted validator.js and validator.min.js to fix merge conflicts, apart from that LGTM 🎉

@profnandaa profnandaa merged commit deb1d1e into validatorjs:master Mar 3, 2021
This was referenced Mar 8, 2021
Closed
Closed
@profnandaa profnandaa mentioned this pull request Apr 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@profnandaa profnandaa profnandaa approved these changes

@tux-tn tux-tn tux-tn approved these changes

Assignees

No one assigned

Labels

🎉 first-pr needs-more-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@varsubham @renanmontebelo @profnandaa @tux-tn