#pr-reviews (2024-09)

approved and merged, thanks @Lennart Goedhart

@Ben Smith (Cloud Posse)

@Ben Smith (Cloud Posse) PR is passed. can you please check now

Merged

@Ben Smith (Cloud Posse)

@Igor Rodionov

I guess these got pulled in during the merge from main, I’ve updated them regardless as its a trivial update

@Ben Smith (Cloud Posse) @Igor Rodionov bumping this up

Is there a step I have missed here? Looks like the terratest has not completed but I am not sure why

Merged. These changes were released in v0.7.0.

Amazing thanks

@Ben Smith (Cloud Posse) @Igor Rodionov

Can someone run the terratest again on following PR

Error: reading Route 53 Hosted Zone (Z04545531TH4B1BZM9PY8): couldn’t find resource https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1099 │ https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1100 │ with module.redis.module.dns.aws_route53_record.default[0], https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1101 │ on .terraform/modules/redis.dns/main.tf line 15, in resource “aws_route53_record” “default”: https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1102 │ 15: resource “aws_route53_record” “default” { https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1103 │ https://github.com/cloudposse/terraform-aws-elasticache-redis/actions/runs/10830359174/job/30049986131#step<i class="em em-12"1104 ╵}

@Ben Smith (Cloud Posse) @Igor Rodionov

@Igor Rodionov is this due to a missing zone in the new test account maybe?

Nitin, we are chasing this internally and will get back to you asap

@Nitinthat merged. The problem was because we had to update the PR base branch.

@Nitin it seems to be failing Terratest

@Gabriela Campana (Cloud Posse) i checked. but this error not with the changes of my code.. can you help me with it.?

@Igor Rodionov is this a test failing due to opentofu / terraform concurrency?

Nitin, we are chasing this internally and will get back to you asap

@Igor Rodionov bumping this up

@Gabriela Campana (Cloud Posse) any update on this

@Igor Rodionov @Andriy Knysh (Cloud Posse)

@Nitin I was working on triaging this bug last week. Still in progress. ETA this week. WIll keep you updated

@Nitin good news. I fixed the tests. So there is no technical block for your PR. Just one point in the PR looks hard for me. Could we simplify the code I mentioned in the PR comments? https://github.com/cloudposse/terraform-aws-vpc-peering-multi-account/pull/101#pullrequestreview-2328471273 ?

@Igor Rodionov can you test now

i have made the changes

@Igor Rodionov can you please rerun

@Nitin thanks. Your PR merged https://github.com/cloudposse/terraform-aws-vpc-peering-multi-account/releases/tag/v0.20.1

@Erik Osterman (Cloud Posse) terratest is passed.

@Gabriela Campana (Cloud Posse) can you please assign someone for approval and merge

@Andriy Knysh (Cloud Posse)

@Igor Rodionov

Merged

These changes were released in v0.19.4.

@Gabriela Campana (Cloud Posse) https://github.com/cloudposse/terraform-aws-elasticache-memcached/issues/55

@Igor Rodionov

@Igor Rodionov @Ben Smith (Cloud Posse)

This PR has been approved (thanks!) but not yet merged.

@Igor Rodionov

because tests are failing

Tests weren’t even run before, so I think it was just that no-one with permissions had run /terratest

And since this is a doc change only, it seems unlikely to be related to a terratest failure, but I can definitely take a look at it.

@Igor Rodionov aware

@Igor Rodionov cc @Gabriela Campana (Cloud Posse)

@Igor Rodionov bumping this up

@Michal Tomaszek thanks for your contribution. We merged it and released at

https://github.com/cloudposse/github-action-atmos-affected-stacks/releases/tag/v5.2.0

@Igor Rodionov I noticed the comments are not on the PRs with the release version

@Erik Osterman (Cloud Posse) yea. For GHA I merge PRs into temporary branch and run test for temporary PR.. that’s why. Actually I think we need for GHA tests use the same pattern we use for terraform modules testing. But I had not time to update workflows yet

approved, thanks @Nitin

These changes were released in v1.6.0.

@Jeremy G (Cloud Posse) @Jeremy White (Cloud Posse)

doing that now

Sorry got pulled away by more things. But the PR is approved and all green. @Tyler Rankin, can you merge?

Looks like there are still a couple workflows that need an approval to run

Awesome, looks like the final workflow checks ran and passed.

@Jeremy White (Cloud Posse) I don’t have write access so it doesn’t let me merge but it looks like it should be good to go!

@Jeremy White (Cloud Posse) bumping this up Also tagging @Ben Smith (Cloud Posse), who might be able to merge it

Not sure if/when mergify might close this again, so just wanted to send a friendly bump here

Ben just merged it now. Thanks!

@RB just let me know I need to update docs, let me do that

Fixed the documents. Sorry about that

@Igor Rodionov can you review this?

@Ben Smith (Cloud Posse)

@Lennart Goedhart, thanks for the contribution. Your PR merged and released https://github.com/cloudposse/terraform-aws-rds-cluster/releases/tag/v1.12.0