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
Meta no longer clones - fixes #233 #311
base: main
Are you sure you want to change the base?
Conversation
@@ -50,7 +50,7 @@ async fn main() -> anyhow::Result<()> { | |||
info!( | |||
"Deleting {}: ({:?})", | |||
Meta::name(&o), | |||
o.status.unwrap().conditions.unwrap().last() | |||
o.status.as_ref().unwrap().conditions.as_ref().unwrap().last() |
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.
This is the kind of awkwardness I am talking about ^
kube/src/api/metadata.rs
Outdated
fn name(&self) -> &String; | ||
/// The namespace the resource is in | ||
fn namespace(&self) -> Option<String>; | ||
fn namespace(&self) -> &Option<String>; | ||
/// Tthe resource version | ||
fn resource_ver(&self) -> Option<String>; | ||
fn resource_ver(&self) -> &Option<String>; |
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.
Should be &str
and Option<&str>
, respectively. That should cut down on the awkward as_ref()
s, and Option<&T>
is covered by the null pointer optimization.
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.
Pushed a change now that tries to do that signature. It shifts things a lot, but it doesn't feel any less awkward. If anything it feels more awkward internally.
As this is now causing more congestion on shared references (as seen in the examples), it makes it awkward when used with option heavy k8s_openapi.
If k8s_openapi implements Arnavion/k8s-openapi#72 then this will feel a lot less awkward and should almost certainly be merged then.
At any rate, wanted to see how this felt. Going to leave this one open for discussion for a bit.