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

PascalCase conversion on structs in codegen #15

Closed
clux opened this issue Nov 3, 2021 · 2 comments
Closed

PascalCase conversion on structs in codegen #15

clux opened this issue Nov 3, 2021 · 2 comments
Labels
question Further information is requested wontfix This will not be worked on

Comments

@clux
Copy link
Member

clux commented Nov 3, 2021

Prost also converts aggressive PascalACRONYMCase names (with allcaps somewhere inside) like APIService to ApiService.
In general, this is a good improvement to the rust conventions, but, like #2, it has some problems:

1. have to disambiguate / break names used in kube

  • ApiResource is an abstraction for APIResource in discovery/core
  • ApiGroup similarly similarly for APIGroup in discovery

2. we break even further from official names

it's already a bit awkward that we convert their camelCased variable names to snake_case, this will also break some of the main structs.


personally, i don't really think 2. is a problem (everyone has been happy with the snake_case conversion in in k8s-openapi despite my earlier reservations), and this extra change only affects a small amount of structs who uses a decidedly bad naming convention.

@clux clux added the question Further information is requested label Nov 3, 2021
clux added a commit that referenced this issue Dec 19, 2023
Mostly a clean bump one minor case overrides for #15 -> `ServiceCIDR`
@clux clux added the wontfix This will not be worked on label Dec 19, 2023
@clux
Copy link
Member Author

clux commented Dec 19, 2023

As an update, here's the small list of overrides currently:

let spec = spec
.replace("APIService", "ApiService")
.replace("CSIDriver", "CsiDriver")
.replace("ClusterCIDR", "ClusterCidr")
.replace("ServiceCIDR", "ServiceCidr")
.replace("IPAddress", "IpAddress")
.replace("CSINode", "CsiNode");
let spec = spec.split("::").map(|m| format_ident!("{}", m));
quote! {
#tokens
impl crate::HasSpec for #type_name {
type Spec = crate::#(#spec)::*;
fn spec(&self) -> Option<&<Self as crate::HasSpec>::Spec> {
self.spec.as_ref()
}
fn spec_mut(&mut self) -> Option<&mut<Self as crate::HasSpec>::Spec> {
self.spec.as_mut()
}
}
}
} else {
tokens
};
let tokens = if let Some(status) = &resource.status {
// HACK Prost changes message name `APIService` to `ApiService`, and `transformed.json` uses the original name.
let status = status
.replace("APIService", "ApiService")
.replace("ServiceCIDR", "ServiceCidr");

This is clearly not a big problem. The only actual question here is what to do about ApiResource and ApiResourceList interface in kube later, and that can be solved there by use k8s-pb::ApiResource as pbApiResource and possibly some features around discovery and the struct source (which we will likely need anyway).

Closing this as wontfix.

@clux clux closed this as completed Dec 19, 2023
@clux
Copy link
Member Author

clux commented Oct 13, 2024

Have undone this in #49 because it would be somewhat annoying to have to keep track of these names upstream in the event of a swithover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant