-
-
Notifications
You must be signed in to change notification settings - Fork 15
feat(stackable-versioned): Generate downgrade From impls #1033
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
base: main
Are you sure you want to change the base?
Conversation
53fe2c7
to
3fc0b82
Compare
|
||
k8s-openapi.workspace = true | ||
kube.workspace = true |
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.
TODO: Check if this is needed
@@ -240,13 +240,13 @@ impl CommonItemAttributes { | |||
|
|||
// The convert_with argument only makes sense to use when the |
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.
// The convert_with argument only makes sense to use when the | |
// The upgrade_with argument only makes sense to use when the |
@@ -240,13 +240,13 @@ impl CommonItemAttributes { | |||
|
|||
// The convert_with argument only makes sense to use when the | |||
// type changed | |||
if let Some(convert_func) = change.convert_with.as_ref() { | |||
if let Some(upgrade_func) = change.upgrade_with.as_ref() { | |||
if change.from_type.is_none() { | |||
errors.push( | |||
Error::custom( | |||
"the `convert_with` argument must be used in combination with `from_type`", |
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.
"the `convert_with` argument must be used in combination with `from_type`", | |
"the `upgrade_with` argument must be used in combination with `from_type`", |
@@ -240,13 +240,13 @@ impl CommonItemAttributes { | |||
|
|||
// The convert_with argument only makes sense to use when the | |||
// type changed | |||
if let Some(convert_func) = change.convert_with.as_ref() { | |||
if let Some(upgrade_func) = change.upgrade_with.as_ref() { |
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.
we need the same code block for downgrade pls
)] | ||
#[serde(rename_all = "camelCase")] | ||
pub struct #status_ident { | ||
pub crd_values: #versioned_crate::CrdValues, |
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.
TODO: Think about a better name
pub status: #status, | ||
} | ||
}), | ||
None => Some(quote! { |
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.
We should combine the variants and only conditionally have
#[serde(flatten)]
pub status: #status,
@@ -391,4 +465,52 @@ impl Struct { | |||
_ => None, | |||
} | |||
} | |||
|
|||
pub fn generate_kubernetes_status_struct(&self) -> Option<TokenStream> { | |||
match &self.common.options.kubernetes_options { |
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.
let kubernetes_options = self.common.options.kubernetes_options.as_ref()?;
instead pls
let status = if multiple_versions { | ||
let status_ident = format_ident!( | ||
"{struct_ident}Status", | ||
struct_ident = self.common.idents.kubernetes.as_ident() | ||
); | ||
Some(quote! { , status = #status_ident }) | ||
} else { |
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.
I would prefer to either wait with the merge until the Status structure is proven to be working or comment this code out or put it behind an opt-in
This PR adds support to downgrade CRDs.
#[versioned()]
macro generatesFrom
impls for downgrading.convert_with
parameter of thechanged()
action accordingly to support customizing both the upgrade and downgrade conversion function. There are now two separate parameters:upgrade_with
anddowngrade_with
.