#pr-reviews (2021-07)
Pull Request Reviews for Cloud Posse Projects
2021-07-16
In need of some help moving https://github.com/cloudposse/terraform-aws-sns-topic/pull/34 forward …
It was previously opened under #32 but because I messed up fork rebasing badly by pointing it to wrong cloudposse repo when adding upstream git remote, I had to reopen it after sorting out our fork….
Updated PR to fix tests and docs …
Also need help moving https://github.com/cloudposse/terraform-aws-sns-topic/pull/35 forward …
2021-07-19
Ok, I am now waiting for some more and from administrators on the above 2 PRs …
i’ll review later today, thanks @azec
thanks!
@Andriy Knysh (Cloud Posse), thank you for the reviews. I have addressed comments on #35 and commented more on #34:
@Andriy Knysh (Cloud Posse) & @jose.amengual, thanks for helping merge #35 .
I have rebased , reworked #34 based on your comments: https://github.com/cloudposse/terraform-aws-sns-topic/pull/34
@azec thanks again, reviewed
@Andriy Knysh (Cloud Posse), I have made updates again, please check it out: https://github.com/cloudposse/terraform-aws-sns-topic/pull/34/files
approved, thanks @azec
Thank you again!
2021-07-20
Hello guys ! Could somebody take time for a quick review of this MR I did a moment ago ? https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/pull/182 It has been ready for a while and is now well tested on my side. Only a strange bridgecrew error failing but with no error.
@Florian SILVA Sorry it’s small but here is the thread with BridgeCrew regarding why they’re failing PR checks.
The tl;dr; is that BridgeCrew fails when the PR is from a fork (which is all PRs that don’t come from folks in this channel, so 99% of em).
Hey gang, made a PR to aws-ecs-web-app which enables setting a custom policy/role for the tasks. Please when you can! TY https://github.com/cloudposse/terraform-aws-ecs-web-app/pull/155
Hey @Matt Gowie make readme succeeded this time
My Go setup on personal machine wasn’t working right, ran it in docker and it’s good to go now
Merged
2021-07-21
2021-07-22
2021-07-28
hi all, please could I have on my PR? https://github.com/cloudposse/terraform-aws-transit-gateway/pull/11 @Andriy Knysh (Cloud Posse) @Maxim Mironenko (Cloud Posse) this is my first contribution of a few in this repo and I may need some input on my follow on (specifically how to achieve multiple route tables/associations) if you have some time please?
@Andriy Knysh (Cloud Posse) @RB the failing test is a problem that will prevent the merge. I’ve seen this a few times now in my local testing. The tgw id exists but the API calls in the AWS provider barfs. What are your thoughts on how to proceed please?
This might be related, but this failing test is run in a single account. https://github.com/hashicorp/terraform-provider-aws/issues/10025
I pushed a potential fix after reviewing that last link
@Andriy Knysh (Cloud Posse) @RB I think this PR is just waiting the final nod before merge. Anything else outstanding?
2021-07-29
Hi, looking for review https://github.com/cloudposse/terraform-aws-eks-cluster/pull/123. This is quite crucial, as kube_exec_auth method doesn’t work with latest AWS CLI v1 without this change.