Uh oh!
There was an error while loading. Please reload this page.
✨ convert field CRDUpgradeSafetyPolicy to be an enum - #1159
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #1159 +/- ##
=======================================
Coverage 75.45% 75.45% =======================================
Files 35 35 Lines 1919 1919 =======================================
Hits 1448 1448 Misses 329 329 Partials 142 142
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
yashoza19
commented
Aug 21, 2024
@everettraven Do you know why this is failing the apidiff check? Is there something I'm missing? |
everettraven
commented
Aug 21, 2024
@yashoza19 this is a breaking change to the API surface. I would expect that check to fail in this case. It is not a required check, but one that is in place to grab our attention when API changes are being made. Once we cut our stable v1.0.0 release, we will likely convert it to a required check |
| //+kubebuilder:Required | ||
| // Disabled represents the state of the CRD upgrade safety preflight check being disabled/enabled. | ||
| Disabled bool `json:"disabled,omitempty"` | ||
| // +kubebuilder:validation:Enum:="Enabled,Disabled" |
There was a problem hiding this comment.
This would make the only allowed value the exact string Enabled,Disabled. I think we are looking for:
| // +kubebuilder:validation:Enum:="Enabled,Disabled" | |
| // +kubebuilder:validation:Enum:="Enabled","Disabled" |
There was a problem hiding this comment.
Good catch. I've updated this in the new commit. Thanks
| Disabled bool `json:"disabled,omitempty"` | ||
| // +kubebuilder:validation:Enum:="Enabled,Disabled" | ||
| // policy represents the state of the CRD upgrade safety preflight check. Allowed values are "Enabled", and Disabled". | ||
| Policy CRDUpgradeSafetyPolicy `json:"policy,omitempty"` |
There was a problem hiding this comment.
This is a required field so we should be good to remove the omitempty here.
| safety preflight check. Allowed values are "Enabled", and | ||
| Disabled". | ||
| enum: | ||
| - Enabled,Disabled |
There was a problem hiding this comment.
Need to regenerate the CRD to get the distinct allowed values here
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: yashoza19 <yoza@redhat.com>
everettraven
left a comment
There was a problem hiding this comment.
Changes look good to me! Thanks @yashoza19 !
Description
Reviewer Checklist