pkg/generator: Create generic file rendering function#310
pkg/generator: Create generic file rendering function#310theishshah merged 13 commits intooperator-framework:masterfrom
Conversation
|
This had been tested against the memcached user guide example and appears to be in working order |
|
So generator.go still needs to be cleaned up, but this offers a single render function instead of the 9 separate files and functions which did functionally the same work of loading a template and replacing values from a struct |
theishshah
changed the title
theishshah
changed the title
|
Fixed issue with creating olm directory |
rithujohn191
left a comment
rithujohn191
left a comment
There was a problem hiding this comment.
Overall looks good. Just a few nits
| return nil | ||
| } | ||
|
|
||
| func renderGenericFile(w io.Writer, fileLoc string, fileTmpl string, info tmplData) error { |
There was a problem hiding this comment.
Could you rename this to renderFile and add a comment above it describing what it does?
| CatalogVersion string | ||
| } | ||
|
|
||
| func (g *Generator) generateDirStructure() error { |
There was a problem hiding this comment.
Could you please add a small comment describing what the function does.
| return writeFileAndPrint(filepath.Join(stubDir, handler), buf.Bytes(), defaultFileMode) | ||
| } | ||
|
|
||
| type tmplData struct { |
There was a problem hiding this comment.
Most of these fields are pretty self-explanatory. But for fields like KindSingular and KindPlural could you add a comment please
There was a problem hiding this comment.
@hasbro17 I wasn't here when the interfaces were designed, can you provide some insight as to why we have 3 versions of Kind?
There was a problem hiding this comment.
It's just the Kubernetes API convention for all types. For e.g a pod has the Kind Pod which defines the scheme it uses. The singular for pods is pod which is an alias for the Kind. And the plural is pods which is used in the REST pattern for the resource.
See https://kubernetes.io/docs/tasks/access-kubernetes-api/extend-api-custom-resource-definitions/#create-a-customresourcedefinition
and https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md
You can just use the comments from the CRD doc if needed:
// plural name to be used in the URL: /apis/<group>/<version>/<plural>
KindSingular string
// singular name to be used as an alias on the CLI and for display
KindPlural stringThere was a problem hiding this comment.
Cool, just updated, should be ready to merge after Travis finishes
| buf := &bytes.Buffer{} | ||
| if err := renderMainFile(buf, repoPath, apiVersion, kind); err != nil { | ||
|
|
||
| if err := renderFile(buf, "cmd/<projectName>/main.go", mainTmpl, tmplData{ |
There was a problem hiding this comment.
Can you define the tmplData argument before the function call. It helps to improve readability:
td := tmplData{
OperatorSDKImport: sdkImport,
StubImport: filepath.Join(repoPath, stubDir),
K8sutilImport: k8sutilImport,
SDKVersionImport: versionImport,
APIVersion: apiVersion,
Kind: kind,
}
if err := renderFile(buf, "cmd/<projectName>/main.go", mainTmpl, td); err != nil {
return err
}There was a problem hiding this comment.
Here and in all other places.
| CatalogVersion string | ||
| } | ||
|
|
||
| //Creates all the necesary directories for the generated files |
There was a problem hiding this comment.
Missing space in comment: //Creates ==> // Creates
| return nil | ||
| } | ||
|
|
||
| //Renders a file given a template, and fills in template fields according to values passed in the tmplData struct |
There was a problem hiding this comment.
Missing space in comment: //Renders ==> // Renders
…0-rebase-master Merge upstream tag v1.28.0
3 participants
Issue #303