-
Notifications
You must be signed in to change notification settings - Fork 423
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
✨ Remove 'default' fields from v1beta1 CRDs #481
✨ Remove 'default' fields from v1beta1 CRDs #481
Conversation
I'm looking into ways we can write a unit/integration test for this, similar to |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: munnerz, pwittrock 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 |
In the future, let's not merge things like this w/o tests :-/. |
@munnerz can you do a follow up with a) a warning, and b) tests? |
Hey @munnerz I'm gonna try adding the warning/test mentioned by Solly above if you're not already working on it. Just wanted to check and make sure I'm not stepping on your toes. |
It is not valid to specify
default
field values in v1beta1 CRDs.This pull request strips the generated openapi schema of all instances of
default
being specified.My only issue with this implementation is that it is non-obvious to users that this has happened, which could cause unexpected behaviour.
Fixes #445
related #480