Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for Israel, in isIdentityCard() #1026

Closed
wants to merge 5 commits into from
Closed

add support for Israel, in isIdentityCard() #1026

wants to merge 5 commits into from

Conversation

perimiter
Copy link
Contributor

added the option of IL, in isIdentityCard().

added the option of IL, in isIdentityCard().
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.

You should not change directly /lib content. You need to add your changes to /src/lib/isIdentityCard.js and use the build command to compile your code using babel.
You also need to add tests to your changes and the new locale to README.md file

@profnandaa profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label May 15, 2019
@profnandaa
Copy link
Member

@perimiter -- thanks for the PR! Please address the comments by @tux-tn

@perimiter
Copy link
Contributor Author

i assume i cant update this pull request? any way, i moved the function to the right folder like you asked, and i did build like you asked. i am not sure though what do you mean by tests, could you show me an example?. in any case the function has its own tests if the string is empty or something like.

@tux-tn
Copy link
Member

tux-tn commented May 16, 2019

@perimiter if you want to add commits to this pull request you have to push your changes to your patch-1 branch and by tests i meant adding test cases like this one for isIdentityCard spanish locale

@profnandaa
Copy link
Member

@me-x-mi could you help out here? I see it's getting stale.

@perimiter perimiter closed this Jun 13, 2019
@perimiter perimiter deleted the patch-1 branch June 13, 2019 12:45
@ezkemboi
Copy link
Member

ezkemboi commented Jun 13, 2019

@profnandaa, I will take a look at this one. It seems the PR is closed?

@perimiter
Copy link
Contributor Author

Hi , I had a problem with merging changes so I decided to start over with a new fork that's more updated. I did every thing you instructed here and created a new PR.

@ezkemboi
Copy link
Member

@perimiter, is this the PR 1041 you have created from the new fork?

@perimiter
Copy link
Contributor Author

yes.

@ezkemboi
Copy link
Member

No problem, when done, I will review it and get some feedback on the same.

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.

4 participants