Skip to content

Conversation

@bflad
Copy link
Contributor

@bflad bflad commented Aug 8, 2022

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

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
@bflad bflad added the tech-debt Issues tracking technical debt that we're carrying. label Aug 8, 2022
@bflad bflad added this to the v0.11.0 milestone Aug 8, 2022
@bflad bflad requested a review from a team as a code owner August 8, 2022 15:55
// 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 {
Copy link
Contributor Author

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()
Copy link
Contributor Author

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.

Copy link
Contributor

@bendbennett bendbennett left a 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

@bflad bflad merged commit 0ba16ff into main Aug 8, 2022
@bflad bflad deleted the bflad/365 branch August 8, 2022 16:54
@github-actions
Copy link

github-actions bot commented Sep 8, 2022

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tech-debt Issues tracking technical debt that we're carrying.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants