Skip to content

Conversation

@VenelinMartinov
Copy link
Contributor

@VenelinMartinov VenelinMartinov commented May 29, 2025

Pure refactor. Now that adding methods to the shim interfaces is no longer breaking we can remove some of the unnecessary interface clutter.

The only effective change here is that the SDKv1 now implements HasDefault, which means #3035 will apply to it as well. This is unlikely to have a meaningful impact.

@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented May 29, 2025

@codecov
Copy link

codecov bot commented May 29, 2025

Codecov Report

Attention: Patch coverage is 36.50794% with 40 lines in your changes missing coverage. Please review.

Project coverage is 68.68%. Comparing base (63f29f1) to head (f6dce5d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/tfshim/schema/schema.go 20.00% 8 Missing ⚠️
pkg/pf/internal/schemashim/type_schema.go 25.00% 6 Missing ⚠️
pkg/pf/internal/schemashim/attr_schema.go 0.00% 4 Missing ⚠️
pkg/pf/internal/schemashim/block_schema.go 50.00% 4 Missing ⚠️
pkg/pf/proto/attribute.go 20.00% 4 Missing ⚠️
pkg/pf/proto/block.go 33.33% 4 Missing ⚠️
pkg/tfshim/sdk-v1/schema.go 0.00% 4 Missing ⚠️
pkg/pf/proto/element.go 25.00% 3 Missing ⚠️
pkg/tfgen/generate.go 66.66% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3101      +/-   ##
==========================================
- Coverage   68.74%   68.68%   -0.06%     
==========================================
  Files         336      336              
  Lines       43610    43645      +35     
==========================================
- Hits        29979    29978       -1     
- Misses      11920    11957      +37     
+ Partials     1711     1710       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@VenelinMartinov VenelinMartinov force-pushed the vvm/refactor_shim_internal branch from d0ef58b to 5fa7b74 Compare May 29, 2025 13:14
@VenelinMartinov VenelinMartinov force-pushed the vvm/clean_up_unnecessary_shim_interfaces branch from ba5a08d to 6259947 Compare May 29, 2025 13:14
return s.tf.DefaultValue()
}

func (s v1Schema) HasDefault() bool {
Copy link
Member

Choose a reason for hiding this comment

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

This is a behavior change for SDK-v1 providers right? Perhaps worth calling out? Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, fair.


func (a attribute) SetHash(v interface{}) int { panic("UNIMPLIMENTED") }
func (a attribute) SetHash(v interface{}) int { panic("UNIMPLIMENTED") }
func (a attribute) SetElementHash(v interface{}) (int, error) { panic("UNIMPLIMENTED") }
Copy link
Member

Choose a reason for hiding this comment

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

How do we know this is not getting called? Perhaps a quick comment?

@t0yv0 t0yv0 self-requested a review May 29, 2025 13:27
@t0yv0 t0yv0 changed the title Clean up unnecessary shim interfaces Required properties with default will now project as optional for SDKv1 providers May 29, 2025
Base automatically changed from vvm/refactor_shim_internal to main June 3, 2025 16:30
@VenelinMartinov VenelinMartinov force-pushed the vvm/clean_up_unnecessary_shim_interfaces branch from 8d6a25c to f6dce5d Compare June 3, 2025 16:30
@VenelinMartinov VenelinMartinov enabled auto-merge (squash) June 3, 2025 16:30
@VenelinMartinov VenelinMartinov merged commit 92748b5 into main Jun 3, 2025
136 checks passed
@VenelinMartinov VenelinMartinov deleted the vvm/clean_up_unnecessary_shim_interfaces branch June 3, 2025 16:56
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v3.110.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants