Skip to content

pkg/generator: final cleanup of generator.go#313

Merged
theishshah merged 4 commits intooperator-framework:masterfrom
theishshah:clean-gen-file
Jun 14, 2018
Merged

pkg/generator: final cleanup of generator.go#313
theishshah merged 4 commits intooperator-framework:masterfrom
theishshah:clean-gen-file

Conversation

@theishshah
Copy link
Copy Markdown
Contributor

Issue #303

@theishshah
Copy link
Copy Markdown
Contributor Author

Tested by following user-guide.md and ensuring proper file generation and directory structure

if err := renderFile(buf, "codegen/update-generated.sh", updateGeneratedTmpl, ugTd); err != nil {
return err
}
return writeFileAndPrint(filepath.Join(codegenDir, updateGenerated), buf.Bytes(), defaultExecFileMode)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the file codegen/update-generated.sh to be executable with the defaultExecFileMode. Currently with this PR operator-sdk new ... fails on the code generation step because the codegen script is not executable:

$ operator-sdk new app-operator --api-version=app.example.com/v1 --kind=App
...
(38/39) Wrote github.com/gogo/protobuf@v1.0.0
(39/39) Wrote k8s.io/client-go@kubernetes-1.9.3
Run dep ensure done
Run code-generation for custom resources
Error: failed to perform code-generation for CustomResources: ()

Also side note but it's weird that I didn't get an error message on failing to run the code generation command. But we can look into that later
https://github.com/operator-framework/operator-sdk/blob/master/commands/operator-sdk/cmd/generate/k8s.go#L55-L61

Copy link
Copy Markdown
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@theishshah theishshah merged commit 03f94a2 into operator-framework:master Jun 14, 2018
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 this pull request may close these issues.

2 participants