-
Notifications
You must be signed in to change notification settings - Fork 12
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
update to go 1.23.0, update dependencies #50
Conversation
@@ -14,7 +13,6 @@ import ( | |||
|
|||
var ( | |||
_ = basetypes.StringTypable(&AAPCustomStringType{}) | |||
_ = xattr.TypeWithValidate(&AAPCustomStringType{}) |
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.
This may get the linters to pass, but if this is deprecated, presumably the Validate()
method will stop being called by TF at some point.
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.
what would be the preferred approach for this?
- Ignore lint failure on the deprecated (only failure in this PR is xattr.TypeWithValidate being deprecated)
- Replace deprecated interface with newer xattr.ValidateableAttribute interface or function.ValidateableParameter interface instead.
If we need to implement newer implement I would appreciate guidance on that, as we have to get this merged to proceed with release as soon as we can.
@gravesm @hakbailey @alinabuzachis @abikouo (tagging as I don't have repo access to add reviewers)
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.
Following on Mike's earlier comment, I vote we go ahead with the release as is but create a Jira ticket to deal with the deprecation.
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.
I'll go ahead and approve this to not block releasing, but it's really important that we deal with the deprecation. By removing the type check on the interface we've removed the only warning we're going to get about this.
Backport to stable-1: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply b8f9e66 on top of patchback/backports/stable-1/b8f9e666b5e06649fe8f308591ef344decc2434d/pr-50 Backporting merged PR #50 into main
🤖 @patchback |
* update to go 1.23.0, update dependencies * update go version in workflows * update golangci-lint-action * update actions/checkout, actions/setup-go * linter fix * remove deprecated xattr.TypeWithValidate() interface (cherry picked from commit b8f9e66)
* update to go 1.23.0, update dependencies * update go version in workflows * update golangci-lint-action * update actions/checkout, actions/setup-go * linter fix * remove deprecated xattr.TypeWithValidate() interface (cherry picked from commit b8f9e66)
actions/checkout@v4
,actions/setup-go@v5
,golangci/golangci-lint-action@v6
xattr.TypeWithValidate()
Imo, we can safely remove
xattr.TypeWithValidate
as func (t AAPCustomStringType) Validate(_ context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { implements the type validation