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

update to go 1.23.0, update dependencies #50

Merged
merged 6 commits into from
Jan 15, 2025
Merged

Conversation

mandar242
Copy link
Contributor

@mandar242 mandar242 commented Jan 14, 2025

  • Update Go version to 1.23.0
  • Update Go version in workflows to 1.23.0
  • Update linter workflow to use latest stable actions/checkout@v4, actions/setup-go@v5, golangci/golangci-lint-action@v6
  • linter fixes to pass ci linter, including removal of deprecated interface 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

@mandar242 mandar242 mentioned this pull request Jan 14, 2025
@@ -14,7 +13,6 @@ import (

var (
_ = basetypes.StringTypable(&AAPCustomStringType{})
_ = xattr.TypeWithValidate(&AAPCustomStringType{})
Copy link
Member

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.

Copy link
Contributor Author

@mandar242 mandar242 Jan 15, 2025

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?

  1. Ignore lint failure on the deprecated (only failure in this PR is xattr.TypeWithValidate being deprecated)
  2. 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)

Copy link
Contributor

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.

Copy link
Member

@gravesm gravesm left a 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.

@jillr jillr merged commit b8f9e66 into ansible:main Jan 15, 2025
8 checks passed
Copy link

patchback bot commented Jan 16, 2025

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

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible/terraform-provider-aap.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-1/b8f9e666b5e06649fe8f308591ef344decc2434d/pr-50 upstream/stable-1
  4. Now, cherry-pick PR update to go 1.23.0, update dependencies #50 contents into that branch:
    $ git cherry-pick -x b8f9e666b5e06649fe8f308591ef344decc2434d
    If it'll yell at you with something like fatal: Commit b8f9e666b5e06649fe8f308591ef344decc2434d is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x b8f9e666b5e06649fe8f308591ef344decc2434d
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR update to go 1.23.0, update dependencies #50 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-1/b8f9e666b5e06649fe8f308591ef344decc2434d/pr-50
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

mandar242 added a commit to mandar242/terraform-provider-aap that referenced this pull request Jan 16, 2025
* 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)
gravesm pushed a commit that referenced this pull request Jan 17, 2025
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants