-
Notifications
You must be signed in to change notification settings - Fork 41
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: Move *Info
objects out of /pkg/tfbridge
to their own package: /pkg/tfbridge/info
#1879
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1879 +/- ##
=======================================
Coverage 60.06% 60.06%
=======================================
Files 323 327 +4
Lines 43894 43919 +25
=======================================
+ Hits 26363 26381 +18
- Misses 16040 16047 +7
Partials 1491 1491 ☔ View full report in Codecov by Sentry. |
Could you give an example use case of this? Would the bridge depend on a specific provider? |
The bridge would not depend on any specific provider. The motivating use case is introducing a new package: |
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.
lgtm
Looking at this before my conclusion was that we inevitably will need to break out the tfbridge package or else all new code has to live in the same package (pretty much), and that Go does not make it easy, but I hesitated to do so as not to cause irritation and extra work for our consumers the link-depend on this code such as Cosmic. If there are changes that a provider maintainer needs to take here to accommodate this upgrade, I'd expect the bridge release to have instructions on how to do it. Perhaps that'd be sufficient. Thanks! |
This PR is a refactor to separate out the user's declaration of a provider (via
tfbridge.*Info
structs and the runtime behavior of the provider (via varioustfbridge.*
methods)This PR moves
/pkg/tfbridge.*Info
and supporting types to/pkg/tfbridge/info.*
. This allows finer grain dependency management, and crucially it allows a package to depend on user information/pkg/tfbridge/info
but be importable by the runtime/pkg/tfbridge
. To the greatest degree possible, this is a pure refactor. There are 3 unavoidable breaking changes:tfbridge.MetadataInfo.ExtractRuntimeMetadata
has been moved from a method ontfbridge.MetadataInfo
(nowinfo.Metdata
) to a stand alone function:tfbridge.ExtractRuntimeMetadata
. This function is public only to share with/pkg/tfgen
and I do not expect users will call it. A GH search shows that only the bridge calls this function. I don't expect other repo to be impacted.tfbridge.ProviderInfo.(Must)?TraverseProperties
has been moved from a method to a standalone function:tfbridge.(Must)?TraverseProperties
. A GH code search shows that this is only used inpulumi-gcp
.pulumi-gcp
is the only repo I expect to be impacted.tfbridge.ElementStrategy[T Resource | DataSource] func(tfToken string, elem *T) error
could not be re-exported since go does not allow type aliases for generic types. This type is largely internal to the bridge and a GH code search does not show any external usage. I don't expect any users to be effected here.All other moved types and methods have been back-ported into
tfbridge
to avoid breaking existing providers. Consumers of the bridge shouldn't observe any changes, they are welcome to continue to usetfbridge.ProviderInfo
and it will continue to work exactly as it previously did.This PR consists entirely of name changes. There are no logic changes, alterations, improvements, etc. The bodies of all functions and types are unchanged.