-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 int/intslice flag validation #1261
Conversation
Yeah i like this feature, if the design is approve i can help implementing other validator (string, time date/duration, net.IP) |
07e15eb
to
19e2d82
Compare
This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else. |
Closing this as it has become stale. |
bump
…On Sat, Aug 28, 2021 at 12:07 PM stale[bot] ***@***.***> wrote:
Closed #1261 <#1261>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1261 (comment)>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAYNLZRFJEVYABJL5BSKKJTT7ECS7ANCNFSM42H7Q4RA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant. |
@dearchap (paraphrasing from discussion) This and #1234 are similar ideas, and given that you're actively working on this one and #1234 has been seemingly abandoned, I favor moving forward with this PR. However, I'm concerned about the additional API surface area introduced here, so what do you think about targeting the v3 series and using generics instead? |
@meatballhat Yes that would be fine. However it might be worthwhile to add a light interface with implementation left to users for extending. And most common use cases can be provided with v3 |
This sounds good to me 🎉 |
Will open a new PR with a small validation API |
What type of PR is this?
What this PR does / why we need it:
This PR adds basic support for flag validation.
Which issue(s) this PR fixes:
Fixes #786
Special notes for your reviewer:
Added support ONLY for int and intslice flags. This is for proof of concept. If this design is acceptable I can add support for other types in a similar fashion.
Testing
New tests have been added
Following tests have been updated to include flag validators
Release Notes