-
Notifications
You must be signed in to change notification settings - Fork 229
Add model name generator #563
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
Conversation
052f613
to
9ea3e5e
Compare
6572856
to
d411358
Compare
@BenTheElder @Jefftree @thockin I've moved the codegen part of this into openapi-gen. This is a backward compatible change that enables us to switch away from go package name reflection in k8s. |
Signed-off-by: Joe Betz <jpbetz@google.com>
Signed-off-by: Joe Betz <jpbetz@google.com>
d411358
to
11cd495
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
I am not intimately familair with this generator but this PR all seems reasonable to me :)
"the base Go import-path under which to generate results") | ||
fs.StringVar(&args.OutputFile, "output-file", "generated.openapi.go", | ||
"the name of the file to be generated") | ||
fs.StringVar(&args.OutputModelNameFile, "output-model-name-file", "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not output to the same file?
EDIT: Oh I see. 1 Generator -> 1 file. Probably not worth changing, but IMO outputting to a single file would be cleaner, if we run across similar things in other generators. Unless there's a reason this is better that I am missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one file for the generated openapi definition, and there are N files for all the zz_geneated.model_name.go files, one for each package containing types that need this function added. Those files are very similar to deep copy generated files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OH RIGHT. Nevermind me :)
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpbetz, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This moves from a reflection based approach to OpenAPI model naming to a declarative approach.
OpenAPIModelName()
receiver functions to access OpenAPI model names of API types are generated intozz_generated.model_name.go
files. This allows API authors to declare the desired OpenAPI model packages when the model name derived from the go package path is not desirable.This PR also migrates the kubernetes API to use this generator (all OpenAPI model names remain the same).
For example:
Generates: