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
generate: make CSV generator for Go GVKs package-aware #4445
generate: make CSV generator for Go GVKs package-aware #4445
Conversation
owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com>
/assign @jmrodri |
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
I would've preferred to see unit tests directly for the ast.go file but seems to be covered by the larger definitions_test.
} | ||
|
||
// findPackagePathForSelExpr returns the package path corresponding to the package name used in expr if it exists in im. | ||
func (im importIdents) findPackagePathForSelExpr(expr *ast.SelectorExpr) (pkgPath string) { |
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.
Nit: Not a fan of this method name but I don't have a better option :) so not blocking this PR.
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com> Signed-off-by: Rein van 't Veer <reinvantveer@gmail.com>
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com> Signed-off-by: Rein van 't Veer <reinvantveer@gmail.com>
/cherry-pick v1.3.x |
@estroz: new pull request created: #4480 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com> Signed-off-by: Rein van 't Veer <reinvantveer@gmail.com>
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com> Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com> Signed-off-by: Rein van 't Veer <reinvantveer@gmail.com> Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>
…ework#4445) internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used. Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com>Signed-off-by: rearl <rearl@secureworks.com>
Description of the change:
Motivation for the change: currently the CSV generator indexes types in the struct-field hierarchy by type name only, which is potentially not unique between packages. This PR makes the generator package-aware.
Closes #4409
Closes #3744
/kind bug
Signed-off-by: Eric Stroczynski ericstroczynski@gmail.com
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs