-
Notifications
You must be signed in to change notification settings - Fork 93
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
tfsdk: Additional preparations for multiple schema implementations #440
Conversation
Reference: #365 This is a followup to the initial schema logic migration that tries to further prepare the Go module for multiple schema implementations. - Removes unexported schema-based functions/methods from the `tfsdk` package (`pathMatches` will be handled as part of schema data internalization) - Removes `tftypes.Type` handling from schemas as `(attr.Type).TerraformType()` already does this - Tries to standardize on `Type()` for fetching framework type (`attr.Type`) information from schema types (`Attribute` will get fixed once `tfsdk.Attribute` is removed) - Tries to standardize `Schema` method signatures for framework types/diagnostics and terraform-plugin-go types/errors - Prepares `NestedAttributes` implementations for unit testing across multiple attribute implementations
// AttributeType returns an attr.Type corresponding to the nested attributes. | ||
func (n UnderlyingAttributes) AttributeType() attr.Type { | ||
// Type returns the framework type of the nested attributes. | ||
func (n UnderlyingAttributes) Type() attr.Type { |
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.
This internal, but exported method was not accessible in the previous release, so updating the method name is safe. Future schema implementations will likely hide internal NestedAttributes
details completely.
if attr.GetAttributes() != nil { | ||
attrTypes[name] = attr.GetAttributes().AttributeType() | ||
} | ||
attrTypes[name] = attr.FrameworkType() |
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.
This is a bug fix from #438, but unreleased yet, so no changelog entry necessary.
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.
You are a refactoring machine!
LGTM
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Reference: #365
This is a followup to the initial schema logic migration that tries to further prepare the Go module for multiple schema implementations.
tfsdk
package (pathMatches
will be handled as part of schema data internalization)tftypes.Type
handling from schemas as(attr.Type).TerraformType()
already does thisType()
for fetching framework type (attr.Type
) information from schema types (Attribute
will get fixed oncetfsdk.Attribute
is removed)Schema
method signatures for framework types/diagnostics and terraform-plugin-go types/errorsNestedAttributes
implementations for unit testing across multiple attribute implementations