-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] Add Ulid constraint and validator #38322
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thnks, almost good to me.
->addViolation(); | ||
} | ||
|
||
if (\strlen($value) !== strspn($value, '0123456789ABCDEFGHJKMNPQRSTVWXYZabcdefghjkmnpqrstvwxyz')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
26 !== strspn(
[...]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes more sense to keep \strlen($value)
otherwise a too short ULID but with only valid characters will have violations about being too short AND having invalid characters.
The violation about invalid characters would not be true in this case.
Let me know what you think about it
Everything modified except one part which I think makes more sense how it is right now (cf comment in code review) |
6493bc9
to
e36fd55
Compare
Thank you @laurent35240. |
…ent35240) This PR was merged into the 5.x branch. Discussion ---------- [Validator] Add documentation for ULID validator Resolves #14298 Related to feature PR symfony/symfony#38322 Commits ------- b37ca01 [Validator] Add documention for ULID validator
ULID constraint and validator as mentioned in ticket #38103
I checked for it ulid specifications https://github.com/ulid/spec