-
Notifications
You must be signed in to change notification settings - Fork 258
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
Refactor identifier logic in pkg/generate/code
#922
Labels
kind/enhancement
Categorizes issue or PR as related to existing feature enhancements.
Comments
brycahta
added
the
kind/enhancement
Categorizes issue or PR as related to existing feature enhancements.
label
Aug 30, 2021
ack-bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this issue
Aug 31, 2021
Issue #, if available: [#922](aws-controllers-k8s/community#922) Description of changes: * Moving 2 helper funcs to become *methods* of CRD Struct for clearer encapsulation: * `GetSanitizedMemberPath` * `GetIdentifiers` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
This was referenced Aug 31, 2021
ack-bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this issue
Sep 1, 2021
Issue #, if available: [#922](aws-controllers-k8s/community#922) Description of changes: * Increasing test coverage of identifier helpers By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
ack-bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this issue
Sep 2, 2021
Issue #, if available: [#922](aws-controllers-k8s/community#922) Description of changes: * Refactor `setSDKReadMany`: * use *FindPluralizedIdentifiersInShape* helpers to simplify logic * Refactor `setResourceForScalar`: * Remove unused params * Add `targetVar` param to be the variable's fully-qualified name (i.e. fieldPath+varName r.ko.Spec.CacheClusterID) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
All enhancements have been implemented; resolving. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem?
No, this is to consolidate and clean up some recently introduced code that works with shape and model identifiers.
Describe the solution you'd like
crd.go
Move getSanitizedMemberPath and FindIdentifiersInCRD here as member functionsIntroduce GetPrimaryIdentifier member func. Precedence: Arn, Id, NameFindPrimaryIdentifierFieldNames
existscheck.go
Move getSanitizedMemberPath out of herecommon.go
Move Find members relating to CRD tocrd.go
Add more unit tests tocommon_test.go
set_resource.go
Refactor setResourceForScalar() to no longer construct paths, but simply take fully-qualified target var name and the source var name to set it to.users of setResourceForScalar that use CRD members can use getSanitizedMemberPath to construct fully-qualified nameset_sdk.go
Refactor setSDKReadMany() to use refactored identifier logicDescribe alternatives you've considered
Discussed with @RedbackThomson offline
The text was updated successfully, but these errors were encountered: