#pr-reviews (2020-04)
Pull Request Reviews for Cloud Posse Projects
2020-04-01
Would it be possible to get a review of https://github.com/cloudposse/terraform-aws-s3-log-storage/pull/25 ?
@Andriy Knysh (Cloud Posse) looks like a quick one (@Maxim Mironenko (Cloud Posse) is on vacation this week)
1
checking
2020-04-02
Hey any chance I could get a push in the right direction on this PR? https://github.com/cloudposse/terraform-aws-ssm-parameter-store/pull/12
@Mikhail Naletov has joined the channel
Hi! I’d be very glad if somebody took a look at my PRs
https://github.com/cloudposse/terraform-aws-elastic-beanstalk-environment/pull/108 https://github.com/cloudposse/terraform-aws-elastic-beanstalk-application/pull/19
Thanks!
Next week we should be faster at reviewing
sorry for the hold up
2020-04-06
@jose.amengual has joined the channel
Thanks for the reviews
2020-04-08
@Joe Niland has joined the channel
[reposted from #terraform]
Hi @Maxim Mironenko (Cloud Posse) would you mind reviewing this PR? I need a bit of help with the bats tests as well. When you have time, I’ll explain what I tried already. Thanks!! https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/27
sure, whats up with bats?
@Joe Niland give me some context of your problems
Sure, one moment
I copied the tests from another cloudposse module and modified them. They are here: https://github.com/cloudposse/terraform-aws-ec2-bastion-server/pull/27/files#diff-920672592109ed71b04a314cf153cb22 I know I need to add a couple more tests but just getting started.
I run make to run the tests.
All the initial ones are OK until check if terraform code is valid.
This just seems to run terraform validate.
If I run this manually it is returning success.
Once I sort that out, I’ll see if the apply test works.
Should be something silly I guess!
ok, I will pull your branch and test it on my local
Thank you
will let you know
@Maxim Mironenko (Cloud Posse) sorry just pushed a small fix for one of the test conditions
ok
@Joe Niland, please, check for my commit. I’ve fixed validation for example module
@Maxim Mironenko (Cloud Posse) thanks - i’ll check it out
ah I see - the validation was failing for the examples/complete module
@Maxim Mironenko (Cloud Posse) PR is updated
thanks, will check for it
Thanks!
Hi @Maxim Mironenko (Cloud Posse) I was wondering if you have had time to look at this one. Let me know if I can fix anything.
Hi @Joe Niland! Will rise the priority of this one. Looks like the only thing left is to setup pipeline for tests. Should be done shortly. Stay tuned and thanks for reminder!
Thanks @Maxim Mironenko (Cloud Posse)!
2020-04-10
another PR for RDS https://github.com/cloudposse/terraform-aws-rds-cluster/pull/64
any takers ?
anyone @here available ?
@jose.amengual I am on it
awesome sir
2020-04-13
2020-04-14
2020-04-20
@cabrinha has joined the channel
2020-04-22
2020-04-23
I’d love a review of https://github.com/cloudposse/terraform-aws-eks-workers/pull/33 if someone has a few moments. It pretty simple, and just adds the environment value from the label module into the rest of the resources.
2020-04-24
Hey folks — I just opened up Issue #207 on build-harness. Is there a reason ya’ll at CloudPosse are printing that message and not running terraform validate? Wanted to check here before I submitted a PR to fix.
Aha this is due to https://github.com/terraform-providers/terraform-provider-aws/issues/11058 / https://github.com/hashicorp/terraform/issues/21408
That’s a PITA…
Aha — there is a workaround though… providing AWS_DEFAULT_REGION before tf validate allows validate to not throw that error. I’m going to put up a PR to fix…
@Matt Gowie won’t get to this until next week. @Maxim Mironenko (Cloud Posse) working on some other fixes at the moment.
@Erik Osterman (Cloud Posse) No rush at all — I just ran into a handful of CP build-harness/test-harness issues while working on https://github.com/cloudposse/terraform-aws-ssm-parameter-store/pull/12.