-
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: Migrate server to internal/proto6server package #310
Conversation
Reference: #215 This change was accomplished by: - `mkdir internal/proto6server` - `git mv tfsdk/serve* internal/proto6server/` - Replacing `package tfsdk` with `package proto6server` in those moved files - Removing `NewProtocol6Server()`, `Serve()`, and `ServeOpts` in moved files - Adding necessary `tfsdk.` scoping to the now external `tfsdk` package references - Moved necessary unexported methods (e.g. `Schema.tfprotov6Schema()`) to `internal/toproto6` (this package will be expanded further in the future) - Moved attribute plan modification testing (including test plan modifiers) to `internal/proto6server/attribute_plan_modification_test.go` - Moved attribute validation testing (including test validators) to `internal/proto6server/attribute_validation_test.go` - Moved block plan modification testing (including test plan modifiers) to `internal/proto6server/block_plan_modification_test.go` - Moved block validation testing (including test validators) to `internal/proto6server/block_validation_test.go` - Copied tfsdk Config/Plan/State.getAttributeValue methods to internal/proto6server (Config/Plan/State)GetAttributeValue functions temporarily to not export existing methods There are some other planned refactorings in the future for the `internal/proto6server` and `tfsdk` code including: - Creating a shared implementation for the underlying `Config`/`Plan`/`State` details - Creating a `internal/fromproto6` package with protocol version 6 type to framework type conversions - Migrating additional framework type to protocol version 6 code to `internal/toproto6` - Migrating RPC handling into individual code and test files These will be handled separately to reduce review burden, as this change was already very large.
Since the code is now external to the package that declares these as deprecated, staticcheck will raise an issue. This silences those reports, either by denoting that the support is required in the case of Block support, or denoting that the functionality will be removed in the case of ResourceImportStateNotImplemented().
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.
Chunky PR :)
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.
Thank you for the report on how this was created.
It's impractical for me to read through the code changes, but with that list at least it's clear it's all "moving stuff around" to create the new structure.
LGTM
Thank you both! |
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: #215
This change was accomplished by the following (with zero actual logic changes):
mkdir internal/proto6server
git mv tfsdk/serve* internal/proto6server/
package tfsdk
withpackage proto6server
in those moved filesNewProtocol6Server()
,Serve()
, andServeOpts
in moved filestfsdk.
scoping to the now externaltfsdk
package referencesSchema.tfprotov6Schema()
) tointernal/toproto6
(this package will be expanded further in the future)internal/proto6server/attribute_plan_modification_test.go
internal/proto6server/attribute_validation_test.go
internal/proto6server/block_plan_modification_test.go
internal/proto6server/block_validation_test.go
There are some other planned refactorings in the future for the
internal/proto6server
andtfsdk
code including:Config
/Plan
/State
detailsinternal/fromproto6
package with protocol version 6 type to framework type conversionsinternal/toproto6
These will be handled separately to reduce review burden, as this change was already very large.