-
Notifications
You must be signed in to change notification settings - Fork 41
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
Extra operations on data types? #80
Comments
The types don't have setters or builders because all the members are |
@Arnavion Oh, I realize they are pub. But unfortunately that doesn't make them very ergonomic. Which is not the end of the world, but is making them inconvenient to work with directly. |
I don't follow what setters and builders would allow you to do that setting the fields directly does not. |
They would allow writing this: let pod_spec = PodSpec::default()
.set_restart_policy("Always")
.append_container(get_default_node_js_container_spec())
.override_with(overrides.generic_pod_overrides)
.override_with(overrides.node_js_pod_overrides); and similar for any arbitrary resource type. |
And the reason you can't set |
It doesn't compose well.
vs ...
? It's not a huge deal, unless you want to use Rust code directly as a semi-DSL, where such code is going to be common and writen by hand a lot. And the |
To be honest, One place where the current API does break down is with manipulating the deeply nested structures that K8s is so fond of, but I don't see how this API would make that cleaner, aside from maybe flattening down /all/ the operations to the root objects... |
I'm trying to design some form of Rust quasi-DSL, that could be used as a helm replacement for the end user to specify a configuration in. As is RN, it's not very pleasant, and having to name and repeat everything over and over is really meh. I totally understand that compilation time etc. might be a big downside, and I'm fine making this a fork/separate crate. E.g. I could make a If you could tell me which parts should I reuse exactly and any tips how to get it done, that could help me a lot. Thanks! |
|
Thanks! |
Stumbling across this: Could this be an extra non-default feature that uses typed-builder/derive_builder to provide builder methods for people that need/want it? |
I think most of those people will stop wanting it after they see the crate takes 50% more time to compile when they enable the feature. That's why this crate doesn't use |
I can only speak for myself obviously but I don't care about extra compile times. Due to the compiler cache I don't really mind if a dependency takes longer. But I agree that this will probably be different for many so I thought having it as an optional feature would be a good compromise. Is this something you'd consider accepting PRs for? |
Yes, but please make sure to add tests for it because it wouldn't get exercised otherwise. |
I really need this functionality now, so I'm working on it in #120 . I still need to improve on it. Hopefully I can get it landed behind a cfg flag. |
I'm kicking the tires of the PR I've opened. Some thoughts and code samples: rustshop/rustshop#12 |
The deep merge functionality just landed and it satisfies my needs, so closing. |
I'm researching implementing a kind-of-helm-replacement that would generate input for
kubectl apply -f -
be just executing Rust source code. In such tool I would like to reuse types generates by k8s-openapi to define all the resources. In that respect the Rust code would become somewhat of a semi-DSL.However, the usability of the types generated by
k8s-openapi
is not great, since they are very bare: just the type definition. Ideally I wish they supported builder pattern and maybe things likefn override_with(&self, other: &Self) -> Self
for helm-like value shadowing where useful.To make an example of what I have in mind:
Would that be something that could be part of this crate? Potentially behind a feature flags for all the users that don't need it? Should it be another crate, using extension crates to add these operations?
The text was updated successfully, but these errors were encountered: