Skip to content

feat: added isTaxId#1336

Merged
profnandaa merged 2 commits intovalidatorjs:masterfrom
rubiin:master
Jun 3, 2020
Merged

feat: added isTaxId#1336
profnandaa merged 2 commits intovalidatorjs:masterfrom
rubiin:master

Conversation

@rubiin
Copy link
Copy Markdown
Member

@rubiin rubiin commented May 30, 2020

added a validator to check taxid
addresses #1320

Checklist

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

@rubiin
Copy link
Copy Markdown
Member Author

rubiin commented May 30, 2020

@profnandaa

Copy link
Copy Markdown
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.

Looks good. Just one thing, please rewrite a little to support locales/contry-codes starting with US as default.

@rubiin
Copy link
Copy Markdown
Member Author

rubiin commented Jun 1, 2020

@profnandaa taxid is only valid for US as per wikipedia

@profnandaa
Copy link
Copy Markdown
Member

I think other locations have equivalents, perhaps going by a different name... will be better to allow future expansion to other locales.

@rubiin
Copy link
Copy Markdown
Member Author

rubiin commented Jun 1, 2020

sure

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Jun 2, 2020
@rubiin
Copy link
Copy Markdown
Member Author

rubiin commented Jun 2, 2020

@profnandaa i have added locales

@rubiin rubiin requested a review from profnandaa June 2, 2020 15:20
Copy link
Copy Markdown
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 🎉

@profnandaa profnandaa merged commit 8adc4c6 into validatorjs:master Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧹 needs-update For PRs that need to be updated before landing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants