-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
dependencies: upgrading to v0.17.0 of github.com/hashicorp/go-azure-helpers #14060
Conversation
…lpers & v2.8.0 of github.com/hashicorp/terraform-plugin-sdk
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
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.
The schema on some functions and not others i called out in my late review on hashicorp/go-azure-helpers#85 - as well as my concerns with how the pointer functions wording is confusing to read
StateFunc: StateFunc, | ||
DiffSuppressFunc: DiffSuppressFunc, | ||
} | ||
return commonschema.LocationSchema() |
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.
does it make sense to have schema on the function name? other functions in there don't?
return commonschema.Location()
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.
(as discussed) will pick this off OOB in the next helpers release
return commonschema.LocationSchemaOptional() | ||
} | ||
|
||
func SchemaComputed() *pluginsdk.Schema { | ||
return &pluginsdk.Schema{ | ||
Type: pluginsdk.TypeString, | ||
Computed: true, | ||
} | ||
return commonschema.LocationSchemaComputed() | ||
} | ||
|
||
// Schema returns the Schema which should be used for Location fields | ||
// where these are Required and can be changed | ||
func SchemaWithoutForceNew() *pluginsdk.Schema { | ||
return &pluginsdk.Schema{ | ||
Type: pluginsdk.TypeString, | ||
Required: true, | ||
ValidateFunc: EnhancedValidate, | ||
StateFunc: StateFunc, | ||
DiffSuppressFunc: DiffSuppressFunc, | ||
} | ||
return commonschema.LocationSchemaWithoutForceNew() |
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.
and here
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.
As discussed approving for now - will address comments in a future pr
This functionality has been released in v2.85.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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. |
This PR:
github.com/hashicorp/go-azure-helpers
github.com/hashicorp/terraform-plugin-sdk
location
package to use the upstreamcommonschema
common location schemav0.17.0 of GoAzureHelpers intentionally moves some files around so there is a breaking change in here which is why this is pretty large - however I've split this into commits, so commit-by-commit this should be fairly clear.