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

Improve required validator #377 #378

Conversation

vasilich6107
Copy link
Contributor

Connection with issue(s)

Close #377

@vasilich6107
Copy link
Contributor Author

Hi @joanpablo
This is a quick one)

@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.07 🎉

Comparison is base (59fe334) 95.83% compared to head (330a17b) 95.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   95.83%   95.90%   +0.07%     
==========================================
  Files          65       65              
  Lines        1319     1320       +1     
==========================================
+ Hits         1264     1266       +2     
+ Misses         55       54       -1     
Impacted Files Coverage Δ
lib/src/validators/required_validator.dart 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vasilich6107 vasilich6107 force-pushed the bugfix/improve_required_validator_issue_#377 branch from b81b8d7 to 330a17b Compare May 9, 2023 09:21
@joanpablo
Copy link
Owner

Hi @vasilich6107

Thanks for been so diligent and taking care of the comments. In this particular case I'm not sure we want to merge both concepts in the same Validator. You can check my answer in the issue #377

@joanpablo
Copy link
Owner

Maybe creating another validator that serves as an alias to the Validators.minLength(1), something like Validators.isNotEmpty could work out

@laogao
Copy link

laogao commented May 10, 2023

Maybe creating another validator that serves as an alias to the Validators.minLength(1), something like Validators.isNotEmpty could work out

There is one caveat for minLength(1), which concerns numbers. I'd much prefer a NonEmptyValidator to an alias to Validators.minLength(1), and "re-use" the ValidationMessage.required key. (See my comment in #377 .)

@vasilich6107
Copy link
Contributor Author

@joanpablo
no problem

@joanpablo
Copy link
Owner

I will close this PR since the discussion is still open.

@joanpablo joanpablo closed this May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RequiredValidator to flag empty lists and maps in addition to nulls and empty strings
3 participants