[codegen] Convert all jennies to use codegen.AppManifest#1265
[codegen] Convert all jennies to use codegen.AppManifest#1265IfSentient merged 9 commits intomainfrom
codegen.AppManifest#1265Conversation
…d of codegen.Kind.
…nstead of codegen.Kind
| build/operator: | ||
| ifeq ("$(wildcard cmd/operator/Dockerfile)","cmd/operator/Dockerfile") | ||
| docker build -t $(OPERATOR_DOCKERIMAGE) -f cmd/operator/Dockerfile . | ||
| docker build -t $(OPERATOR_DOCKERIMAGE) -f cmd/operator/Dockerfile . |
There was a problem hiding this comment.
Drive-by that I noticed when doing new project testing--this was four spaces instead of a tab.
codegen/jennies/backendplugin.go
Outdated
| for _, version := range appManifest.Versions() { | ||
| if version.Name() != appManifest.Properties().PreferredVersion { | ||
| continue | ||
| } | ||
| for _, kind := range version.Kinds() { | ||
| tmd.Resources = append(tmd.Resources, versionedKindToKindProperties(kind, appManifest)) | ||
| if appManifest.Properties().FullGroup != "" { | ||
| tmd.PluginID = strings.Split(appManifest.Properties().FullGroup, ".")[0] | ||
| } |
There was a problem hiding this comment.
this is somewhat similar to the logic for getting the tmd.Resources from the manifest that you did in app generator. Can we extract this logic and potentially pass the props to the Generate func, or create a helper func that does this? That way we only do one pass through the kinds. It does seem that there are some minute differences that could make this tricky, but just an idea.
There was a problem hiding this comment.
The KindProperties stuff should probably also be overhauled in the future--for now I didn't want to change the templates in this PR.
codegen/jennies/codec.go
Outdated
| for _, version := range appManifest.Versions() { | ||
| for _, kind := range version.Kinds() { |
There was a problem hiding this comment.
just an idea, but could we create an func that returns an iterator to reduce this nested loop, which you use a lot throughout this PR i.e.
| for _, version := range appManifest.Versions() { | |
| for _, kind := range version.Kinds() { | |
| for version, kind := range ManifestKindVersions(appManifest))) { |
func ManifestKindVersions(m AppManifest) iter.Seq2[Version, VersionedKind] {
return func(yield func(Version, VersionedKind) bool) {
for _, version := range m.Versions() {
for _, kind := range version.Kinds() {
if !yield(version, kind) {
return
}
}
}
}
}There was a problem hiding this comment.
Added codegen.VersionedKinds and codegen.PreferredVersionKinds using iter.Seq2. Good suggestion, I keep forgetting about the iterator stuff that go introduced, and this is a great place to use it.
codegen/jennies/codec.go
Outdated
| if kind.Scope != string(resource.NamespacedScope) && kind.Scope != string(resource.ClusterScope) { | ||
| return nil, fmt.Errorf("scope '%s' is invalid, must be one of: '%s', '%s'", | ||
| kind.Scope, resource.ClusterScope, resource.NamespacedScope) | ||
| } |
There was a problem hiding this comment.
nit; maybe you could use a helper func for this scope check?
| @@ -134,10 +146,6 @@ func (o *operatorMainJenny) Generate(kinds ...codegen.Kind) (*codejen.File, erro | |||
| KindsAreGrouped: !o.groupByKind, | |||
| } | |||
|
|
|||
| for _, kind := range kinds { | |||
| tmd.Resources = append(tmd.Resources, kind.Properties()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Why can you just remove this?
There was a problem hiding this comment.
Resources isn't used in the template, I forgot to remove it from the metadata as well, let me fix that.
codegen/jennies/resourceobject.go
Outdated
| files = append(files, codejen.File{ | ||
| RelativePath: filepath.Join(GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName), fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName))), | ||
| Data: b, | ||
| From: []codejen.NamedJenny{r}, | ||
| }) |
There was a problem hiding this comment.
nit; just a little cleaner to follow
| files = append(files, codejen.File{ | |
| RelativePath: filepath.Join(GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName), fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName))), | |
| Data: b, | |
| From: []codejen.NamedJenny{r}, | |
| }) | |
| typePath := GetGeneratedGoTypePath(r.GroupByKind, appManifest.Properties().Group, version.Name(), kind.MachineName) | |
| suffix := fmt.Sprintf("%s_object_gen.go", strings.ToLower(kind.MachineName)) | |
| files = append(files, codejen.File{ | |
| RelativePath: filepath.Join(typePath, suffix), | |
| Data: b, | |
| From: []codejen.NamedJenny{r}, | |
| }) | |
| // Any type parser should be able to parse a kind into this definition to supply | ||
| // to various common Jennies in the codegen package. | ||
| // Deprecated: use AppManifest instead | ||
| type Kind interface { |
There was a problem hiding this comment.
This is still being used in some places, any reason why not removing it across the repo?
There was a problem hiding this comment.
The only place left that still uses it AFAICT are the parsers, which are exported. I'll deprecate the KindParser, but given that it's an exported type, I want ot leave it present and deprecated for a few releases just in case anyone is using it.
…en.Kind, create convenience iterator functions in the codegen package for iterating through versions+kinds in order, other small fixes based on PR comments.
What Changed? Why?
This PR converts all jennies still using
codegen.Kindover to usingcodegen.AppManifest, and deprecatescodegen.Kind. Currently, as we continue to add jennies, they have to end up in generators that are usingcodegen.AppManifest, and don't always fit in that space (see the go client jenny as an example, which is in the manifest generator instead of the resource one that generates all other gotype code). This change allows us to more intelligently group jennies in the future, and move the codegen model over entirely to be AppManifest-based.How was it tested?
Codegen tests to ensure output didn't change, and created and built a new project to make sure the project codegen was still accurate.
Where did you document your changes?
N/A - no user-facing changes.
Notes to Reviewers