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: docs and tests refactoring for minLength #416

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ariskemper
Copy link
Contributor

@ariskemper ariskemper commented Feb 5, 2024

Referencing #396

@fabian-hiller fabian-hiller self-assigned this Feb 5, 2024
@fabian-hiller fabian-hiller added the enhancement New feature or request label Feb 5, 2024
@fabian-hiller
Copy link
Owner

Thank you for creating this issue. I want to figure out the use case first. I am not sure if we should really support length for numbers. Especially for pointing float numbers it can be confusing. I think minValue is the better choice here.

@fabian-hiller fabian-hiller added the question Further information is requested label Feb 5, 2024
@fabian-hiller
Copy link
Owner

It seems like minValue does the job and it is not required to support numbers in minLength.

@ariskemper
Copy link
Contributor Author

ariskemper commented Feb 5, 2024

@fabian-hiller good point, i was not going in detail regarding use case, but probably maxValue, minValue solve this and we keep minLength, maxLength for strings, arrays. I could rename the PRs and we keep the refactored tests and i add the docs in the same PR?

@fabian-hiller
Copy link
Owner

Yes, sounds good! Thank you!

@ariskemper ariskemper marked this pull request as draft February 6, 2024 17:24
@ariskemper ariskemper changed the title feat: add min length for number feat: docs and tests refactoring for minLength Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants