Skip to content

Added countries to isPassportNumber.#1555

Closed
JuanFML wants to merge 2 commits intovalidatorjs:masterfrom
JuanFML:add-countries-passport
Closed

Added countries to isPassportNumber.#1555
JuanFML wants to merge 2 commits intovalidatorjs:masterfrom
JuanFML:add-countries-passport

Conversation

@JuanFML
Copy link
Contributor

@JuanFML JuanFML commented Dec 14, 2020 •
edited
Loading

Enlarged feat isPassportNumber #1288 : Added more countries into the list

Added the next countries with their corresponding tests:

  • Mexico, MX
  • Kazakhstan, KZ
  • Liechtenstein, LI
  • Malaysia, MY
  • Thailand, TH
  • New Zeland, NZ
  • Jamaica, JM

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov
Copy link

codecov bot commented Dec 15, 2020 •
edited
Loading

Codecov Report

Merging #1555 (7529acf) into master (787df19) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1555   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           99        99           
  Lines         1776      1776           
=========================================
  Hits          1776      1776
Impacted Files Coverage Δ
src/lib/isPassportNumber.js 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 787df19...7529acf. Read the comment docs.

tux-tn
tux-tn approved these changes Dec 15, 2020
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.

Congrats @JuanFML for your first PR 🎉
LGTM !

@profnandaa
Copy link
Member

profnandaa commented Feb 15, 2021

This will be merged after our release rework PR #1553

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed 🧹 needs-update For PRs that need to be updated before landing and removed needs-more-review ready-to-land For PRs that are reviewed and ready to be landed labels Mar 12, 2021
tux-tn
tux-tn requested changes Mar 12, 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.

@JuanFML Can you fix conflicts?

@profnandaa
Copy link
Member

profnandaa commented Jul 16, 2021

@JuanFML -- pls fix the merge conflicts and we should be good to go. Thanks for your contribution!

@rubiin
Copy link
Member

rubiin commented Sep 26, 2021

@profnandaa can i work on this as author is awol

@tux-tn tux-tn added the ☹ abandoned PR that is unattended by the author for at least 3 months. label Oct 2, 2021
@ezkemboi
Copy link
Member

ezkemboi commented Oct 8, 2021

@JuanFML can you have some time to polish these merge conflicts and we can get this merged?

@rubiin we just want to get these people credits for the work they have done instead of building it as our own.

@fedeci
Copy link
Contributor

fedeci commented Oct 8, 2021

If the PR is really abandoned, we can still create a new one based on this and add @JuanFML as co-author.

@ezkemboi
Copy link
Member

ezkemboi commented Oct 8, 2021

@fedeci when doing clean up of stale PR's, we don't create new nor delete the existing one.
Please check this guide on how we do clean stale Pr's

https://github.com/validatorjs/validator.js/wiki/Maintenance:-PR-Clean-Up

@fedeci
Copy link
Contributor

fedeci commented Oct 8, 2021 •
edited
Loading

The last step explicitly say create a new PR :)
I think we meant the same process, however.

@ezkemboi
Copy link
Member

ezkemboi commented Oct 8, 2021 via email

@rubiin
Copy link
Member

rubiin commented Oct 18, 2021

@ezkemboi yes we can do this.

@profnandaa profnandaa added the mc-to-land Just merge-conflict standing between the PR and landing. label Dec 12, 2022
profnandaa
profnandaa approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM except for the merge conflicts, will try fix them.

profnandaa added a commit that referenced this pull request Jan 31, 2023
@profnandaa @JuanFML
feat(isPassportNumber): new locales JM,KZ,LI,NZ
391dd82 
chore: fix merge conflicts for #1555

Co-authored-by: JuanFML <[email protected]>
profnandaa added a commit that referenced this pull request Jan 31, 2023
@profnandaa @JuanFML
feat(isPassportNumber): new locales JM,KZ,LI,NZ
8604748 
chore: fix merge conflicts for #1555

Co-authored-by: Juan Medina <[email protected]>
@profnandaa
Copy link
Member

profnandaa commented Jan 31, 2023

included in the combined PR - #2164

@profnandaa profnandaa closed this Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tux-tn tux-tn tux-tn requested changes

@profnandaa profnandaa profnandaa approved these changes

Assignees

No one assigned

Labels

☹ abandoned PR that is unattended by the author for at least 3 months. 🎉 first-pr mc-to-land Just merge-conflict standing between the PR and landing. 🧹 needs-update For PRs that need to be updated before landing PR/combined v-next

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@JuanFML @profnandaa @rubiin @ezkemboi @fedeci @tux-tn