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

Remove deprecated argument "vpc" #230

Merged
merged 6 commits into from
May 10, 2024
Merged

Remove deprecated argument "vpc" #230

merged 6 commits into from
May 10, 2024

Conversation

yuekui
Copy link
Contributor

@yuekui yuekui commented May 7, 2024

what

Remove the deprecated vpc argument to fix the warning. The vpc parameter has been deprecated since the AWS provider version 5.0, there's no need to explicitly specify either vpc or domain. Simply omitting it and using the default value will ensure optimal compatibility.

image

references

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eip#vpc

@yuekui yuekui requested a review from a team as a code owner May 7, 2024 04:14
@mergify mergify bot added the triage Needs triage label May 7, 2024
@yuekui yuekui changed the title Replace deprecated argument with "domain" Replace deprecated argument "vpc" with "domain" May 7, 2024
main.tf Outdated Show resolved Hide resolved
@yuekui yuekui changed the title Replace deprecated argument "vpc" with "domain" Remove deprecated argument "vpc" May 7, 2024
@nitrocode nitrocode added patch A minor, backward compatible change and removed triage Needs triage labels May 7, 2024
@nitrocode
Copy link
Member

/terratest

nitrocode
nitrocode previously approved these changes May 7, 2024
@nitrocode nitrocode enabled auto-merge (squash) May 7, 2024 13:28
@nitrocode
Copy link
Member

I'm unsure on the correct fix in the lint step and the codeowners seems different than other repos which leads me to believe that the repo is a little out of date. I reached out in the sweetops slack channel for assistance by an @cloudposse/admins and @cloudposse/approvers .

https://sweetops.slack.com/archives/CUJPCP1K6/p1715088607211699

@osterman
Copy link
Member

osterman commented May 7, 2024

/terratest

@nitrocode nitrocode mentioned this pull request May 8, 2024
@nitrocode
Copy link
Member

@osterman pr 232 should fix the test in this pr

#232

@osterman
Copy link
Member

osterman commented May 8, 2024

/terratest

@nitrocode
Copy link
Member

Still seems like the linter is failing @osterman. Terratest passes tho

@osterman
Copy link
Member

@goruha can you check what is going on?

@Nuru
Copy link

Nuru commented May 10, 2024

@osterman @goruha As I said before, adding issues: write (#231) does not fix the problem of the actions not being able to post comments on the PRs.

@yuekui @nitrocode The linter is failing because it is applying new standards to old code. Nothing in the PR is wrong, but despite that, syntax in the Terraform code (in the module or the examples) like

join("", aws_instance.default.*.id)

is no longer allowed. You need to globally replace .*. with [*]..

@yuekui
Copy link
Contributor Author

yuekui commented May 10, 2024

@yuekui @nitrocode The linter is failing because it is applying new standards to old code. Nothing in the PR is wrong, but despite that, syntax in the Terraform code (in the module or the examples) like

join("", aws_instance.default.*.id)

is no longer allowed. You need to globally replace .*. with [*]..

Got it, I could fix that, thanks!

Copy link

mergify bot commented May 10, 2024

💥 This pull request now has conflicts. Could you fix it @yuekui? 🙏

@mergify mergify bot added the conflict This PR has conflicts label May 10, 2024
@mergify mergify bot added no-changes No changes were made in this PR and removed conflict This PR has conflicts labels May 10, 2024
@mergify mergify bot closed this May 10, 2024
auto-merge was automatically disabled May 10, 2024 07:40

Pull request was closed

Copy link

mergify bot commented May 10, 2024

This pull request was automatically closed as it no longer contains any changes.

This typically happens when another merged pull request has already included this request's
proposed modifications into the default branch.

@Nuru Nuru reopened this May 10, 2024
@mergify mergify bot closed this May 10, 2024
Copy link

mergify bot commented May 10, 2024

This pull request was automatically closed as it no longer contains any changes.

This typically happens when another merged pull request has already included this request's
proposed modifications into the default branch.

@Nuru Nuru reopened this May 10, 2024
@Nuru Nuru removed the no-changes No changes were made in this PR label May 10, 2024
@Nuru
Copy link

Nuru commented May 10, 2024

/terratest

@Nuru Nuru merged commit eec81d6 into cloudposse:main May 10, 2024
10 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants