Skip to content
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

Mildwonkey/providers interface renaming #27805

Merged
merged 3 commits into from
Feb 18, 2021

Conversation

mildwonkey
Copy link
Contributor

@mildwonkey mildwonkey commented Feb 17, 2021

This PR builds on #27793 and will be merged only after that is reviewed and merged.

This PR renamed several functions and types in providers.Interface, and weaves that change throughout terraform. I put the interface change in a separate commit to aid in reviewing; I will squash these commits together when merging.

This is a purely aesthetic change, updating our internal names to match the new protocol v6. There is no expected impact on providers written against protocol v5 - nothing has changed in the v5 protocol definition, only in the conversion from that package to internal terraform types.

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #27805 (91087fe) into master (2b4e389) will decrease coverage by 0.00%.
The diff coverage is 62.82%.

Impacted Files Coverage Δ
builtin/providers/terraform/provider.go 0.00% <0.00%> (ø)
internal/legacy/terraform/provider_mock.go 0.00% <0.00%> (ø)
...nternal/legacy/terraform/resource_provider_mock.go 0.00% <0.00%> (ø)
internal/legacy/terraform/schemas.go 0.00% <0.00%> (ø)
providers/factory.go 0.00% <0.00%> (ø)
providers/provider.go 0.00% <ø> (ø)
terraform/node_provider.go 87.50% <85.71%> (ø)
terraform/provider_mock.go 69.60% <90.32%> (ø)
backend/local/testing.go 63.55% <100.00%> (ø)
plugin/grpc_provider.go 55.18% <100.00%> (ø)
... and 6 more

@mildwonkey mildwonkey requested a review from a team February 17, 2021 16:34
Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mostly looks good to me! The new names are much clearer.

I think a couple of the renames (in getproviders and the old registry client) may not be right. Can you check?

Base automatically changed from mildwonkey/nested-type-format to master February 18, 2021 13:51
This commit renames a handful of functions in the providers.Interface to
match changes made in protocol v6. The following commit implements this
change across the rest of the codebase; I put this in a separate commit
for ease of reviewing and will squash these together when merging.

One noteworthy detail: protocol v6 removes the config from the
ValidateProviderConfigResponse, since it's never been used. I chose to
leave that in place in the interface until we deprecate support for
protocol v5 entirely.
Several function names and types have change in the providers.Interface.
This commit weaves those changes through all of terraform.

Note that none of these changes impact current providers using protocol
v5; the protocol is unchanged. Only the translation layer between the
proto and terraform have changed.
@mildwonkey mildwonkey force-pushed the mildwonkey/providers-interface-renaming branch from a82d77d to 13b6dab Compare February 18, 2021 13:54
@mildwonkey mildwonkey force-pushed the mildwonkey/providers-interface-renaming branch from 13b6dab to 91087fe Compare February 18, 2021 14:36
@mildwonkey mildwonkey requested a review from alisdair February 18, 2021 14:47
@mildwonkey mildwonkey merged commit f650587 into master Feb 18, 2021
@mildwonkey mildwonkey deleted the mildwonkey/providers-interface-renaming branch February 18, 2021 15:13
@ghost
Copy link

ghost commented Mar 21, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants