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

feat: ISIN validation #298

Closed
wants to merge 3 commits into from
Closed

Conversation

ariskemper
Copy link
Contributor

@ariskemper ariskemper commented Dec 15, 2023

  • Validation for ISIN ( Internatinal Securities Identification Number ) with format validation and use of luhn algorithm

Copy link

netlify bot commented Dec 15, 2023

‼️ Deploy request for valibot rejected.

Name Link
🔨 Latest commit 1c39b51

@fabian-hiller fabian-hiller self-assigned this Dec 15, 2023
@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority labels Dec 15, 2023
@fabian-hiller
Copy link
Owner

Thank you so much for all the work you put into Valibot! It means a lot to me and it provides a great value to all Valibot users. If you have time during the winter break, I would love to get to know you in a video call.

@ariskemper
Copy link
Contributor Author

Sure we could have a call and discuss how we could improve the library.

@fabian-hiller
Copy link
Owner

In case you use Discord, feel free to send me (fabianhiller) a friend request.

@fabian-hiller
Copy link
Owner

Can you check your code against the formatting requirements and this code of validator.js to see if it covers every case?

@ariskemper ariskemper marked this pull request as draft December 23, 2023 11:41
@ariskemper
Copy link
Contributor Author

ariskemper commented Dec 24, 2023

@fabian-hiller i refactor and moved isISIN to validation. I have done some research and most common use is mod10 in validation of ISIN. Mod 10 is simpler to implement and faster to compute, which is often sufficient for shorter sequences like the 12-character ISIN.

@ariskemper ariskemper marked this pull request as ready for review December 24, 2023 12:28
@fabian-hiller
Copy link
Owner

Thanks! Will have a look at it next week.

@fabian-hiller
Copy link
Owner

Unfortunately, due to our rewrite a few months ago, this PR is no longer up to date with our source code. I still plan to add this action, but other things have a higher priority for me at the moment. If you or anyone else in the community is interested in creating a new PR, that would be great.

@fabian-hiller
Copy link
Owner

I will close this PR. However, I will keep it in mind when adding this action in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants