search

kube-rs / kopium

Kubernetes OPenapI UnMangler
Apache License 2.0
112 stars 21 forks source link

Passing `--derive Default` does not derive it for top level custom resource #232

Closed david-rusnak closed 4 months ago

david-rusnak commented 4 months ago

When playing around with Kopium I realized that passing --derive Default derives Default for child structs, but not the top level CustomResource. Adding #kube[(derive="Default")] worked, but I'm not sure this is desired behavior, since I realize that not every user specified trait on the derive field can be added as a #kube[(derive="TRAIT")] without conflicts.

For example, I had to add a conditional to check if I had tried to derive default. https://github.com/kube-rs/kopium/compare/main...david-rusnak:kopium:feat/add-customresource-default-if-specified?expand=1

Again, not sure this is desired, but thought I'd flag it here in case anyone else was seeing this.

clux commented 4 months ago

Yeah, this is one of those awkward trade-offs with having the top level struct generated by proc macro code; anything you need to customise on the struct must be fed in via attributes, in this case #[kube(derive = "Trait")].

My gut feel is that it is worth taking your change and maybe generalising it for the ones we have listed excluding exceptions that don't work. As far as I can tell, the only clash would be JsonSchema, which I believe we add in kube-derive already. Are there others?

david-rusnak commented 4 months ago

@clux Yeah, the only collision I've found was JsonSchema. I've just tried it for the rest of the allowed traits and none of them had impl collisions. I've pushed up a more generalized approach (which you can feel free to review or mess with for formatting - it's possible to define these derives as #kube[(derive="Trait", derive="Trait", etc..)] but I was not sure which format is preferred.)

Thanks for taking a look!

clux commented 4 months ago

I think what you have is great, formatting is the job of rustfmt anyway. Feel free to submit a PR and we can see if CI has any objections.