Skip to content
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

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

Closed
david-rusnak opened this issue Apr 29, 2024 · 3 comments · Fixed by #236
Closed

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

david-rusnak opened this issue Apr 29, 2024 · 3 comments · Fixed by #236

Comments

@david-rusnak
Copy link
Contributor

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
Copy link
Member

clux commented Apr 29, 2024

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
Copy link
Contributor Author

@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
Copy link
Member

clux commented Apr 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants