-
Notifications
You must be signed in to change notification settings - Fork 416
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
🐛 Allow floating-point values in validations #619
🐛 Allow floating-point values in validations #619
Conversation
Hi @Porges. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
5e209fb
to
618e68e
Compare
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
/remove-lifecycle rotten bad bot |
It would be nice to get this merged. It's been open for a while. |
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.
Did a quick scan. Looks reasonable to me, but I am not too familiar with this codebase.
/ok-to-test
Hi, Thanks for your contribution! Overall it looks good to me, I'm just wondering about this "AllowDangerousTypes" that you also added in the marker |
/lgtm |
For some nits: But its nothing to block merging over, so: If you want to accept the nits, ping me again and I can re-add lgtm and approve labels. |
Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
It’s required in order to use float values at all in the test file. |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlmogBaku, joelanford, Porges The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Thanks for reviewing @joelanford |
Closes #543. We have also run into this while working on https://github.com/Azure/azure-service-operator.
number
values.Minimum
,Maximum
, andMultipleOf
to have float values when applied to float fields.Actual use of float fields is still restricted by the
allowDangerousTypes
flag, so no further restrictions are needed as part of this change.