#pr-reviews (2020-11)

Pull Request Reviews for Cloud Posse Projects

2020-11-30

roth.andy avatar
roth.andy
Update versions.tf by RothAndrew · Pull Request #106 · cloudposse/terraform-aws-dynamic-subnets

what Increase terraform min version to 0.12.26 due to new required_providers format Relax provider version requirements by using >= instead of ~> why Provide greater compatibility with new…

1

2020-11-24

2020-11-23

Mikhail Naletov avatar
Mikhail Naletov
Latest cloudposse features by okgolove · Pull Request #25 · cloudposse/terraform-aws-alb-target-group-cloudwatch-sns-alarms

what Allow Terraform 0.14 Use gte for providers Use latest label module and context feature why The module should have context feature due to it is used by another modules like ecs-web-app

1
Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

Hey there

Latest cloudposse features by okgolove · Pull Request #25 · cloudposse/terraform-aws-alb-target-group-cloudwatch-sns-alarms

what Allow Terraform 0.14 Use gte for providers Use latest label module and context feature why The module should have context feature due to it is used by another modules like ecs-web-app

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

So this still needs the new test/src/Makefile

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

which is why the terratest is failing

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
okgolove/terraform-aws-alb-target-group-cloudwatch-sns-alarms

Terraform module to create CloudWatch Alarms on ALB Target level metrics. - okgolove/terraform-aws-alb-target-group-cloudwatch-sns-alarms

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
cloudposse/terraform-null-label

Terraform Module to define a consistent naming convention by (namespace, stage, name, [attributes]) - cloudposse/terraform-null-label

Joe Niland avatar
Joe Niland

That’s my mistake. I asked @ to change the tests but forgot about the Makefile!

Latest cloudposse features by okgolove · Pull Request #25 · cloudposse/terraform-aws-alb-target-group-cloudwatch-sns-alarms

what Allow Terraform 0.14 Use gte for providers Use latest label module and context feature why The module should have context feature due to it is used by another modules like ecs-web-app

Mikhail Naletov avatar
Mikhail Naletov
cloudposse/terraform-null-label

Terraform Module to define a consistent naming convention by (namespace, stage, name, [attributes]) - cloudposse/terraform-null-label

okgolove/terraform-aws-alb-target-group-cloudwatch-sns-alarms

Terraform module to create CloudWatch Alarms on ALB Target level metrics. - okgolove/terraform-aws-alb-target-group-cloudwatch-sns-alarms

Joe Niland avatar
Joe Niland

Yes

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
feat: add possability to use lowercased context tags by SweetOps · Pull Request #107 · cloudposse/terraform-null-label

what add possability to use lowercased context tags why not all cloud-providers supports the uppercased keys for tagging/labeling resources

Vladimir S. avatar
Vladimir S.

hey @Erik Osterman (Cloud Posse), sorry I’m currently overloaded on my work… and I looking for new opportunities in career… so interviews eats the rest of time… I hope that I’ll find a couple of hours on this week, I started to work on this on past Friday, but mb 15 mins. I thinking this to implement in style similar to @nuru suggestion. tag_format = "default:titlecase" where default is tags formatting style which is CP style(titlecased keys and lowercased values)

feat: add possability to use lowercased context tags by SweetOps · Pull Request #107 · cloudposse/terraform-null-label

what add possability to use lowercased context tags why not all cloud-providers supports the uppercased keys for tagging/labeling resources

Vladimir S. avatar
Vladimir S.

and also others snake case,camelcase, kebab, etc

Vladimir S. avatar
Vladimir S.

when I’ll have a PoC than we can discuss

Vladimir S. avatar
Vladimir S.

@Erik Osterman (Cloud Posse) I’ve made some PoC and pushed it to same branch(PR) so you can take a look or play with new functionality

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

great! we’ll take a look at this

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

@Jeremy (Cloud Posse)

1
Vladimir S. avatar
Vladimir S.
06:10:45 AM

@Vladimir S. has joined the channel

2020-11-21

Mikhail Naletov avatar
Mikhail Naletov
Use context.tf and latest null-label module by okgolove · Pull Request #9 · cloudposse/terraform-aws-ecs-cloudwatch-autoscaling

what Use context feature and latest null-label module why The module should have context feature due to it is used by another modules like ecs-web-app

1

2020-11-17

Tyrone Meijn avatar
Tyrone Meijn
Use private DNS record when not associating public IP by tyronedd · Pull Request #42 · cloudposse/terraform-aws-ec2-bastion-server

What Use private DNS record when var.associate_public_address = false. Why When not associating public IP the record would be empty. This fixes that.

1

2020-11-16

Mikhail Naletov avatar
Mikhail Naletov
Loosen the AWS provider requirement by okgolove · Pull Request #77 · cloudposse/terraform-aws-ecs-web-app

what Update aws provider version requirements why 3.0 AWS provider is already here and we should be able to use the module with it

Use aws provider 3.x by okgolove · Pull Request #9 · cloudposse/terraform-aws-nlb

what Update aws provider version requirements why 3.0 AWS provider is already here and we should be able to use the module with it

Frank avatar
Frank
Convert to context by syphernl · Pull Request #38 · cloudposse/terraform-aws-ec2-bastion-server

what Converted this module to use context.tf why To follow the new standard applied across the other modules. refs closes #30

Update default AMI ID by syphernl · Pull Request #40 · cloudposse/terraform-aws-ec2-bastion-server

what Update the default / example AMI ID to the most recent version Added an example on how to always obtain the latest AMI ID within the Terraform configuration why Out of the box (without an e…

Convert to context by syphernl · Pull Request #52 · cloudposse/terraform-aws-key-pair

what Converted this module to use context.tf why To follow the new standard applied across the other modules.

Fix flapping security groups by syphernl · Pull Request #39 · cloudposse/terraform-aws-ec2-bastion-server

what Added explicit description to the ingress why AWS stores the unset description as "" but Terraform turns this into a null value, therefor triggering to make changes. Example: #…

2020-11-12

Mikhail Naletov avatar
Mikhail Naletov

Hi! I’d like to user 3.x aws provider with ecs module https://github.com/cloudposse/terraform-aws-ecs-web-app/pull/77

Loosen the AWS provider requirement by okgolove · Pull Request #77 · cloudposse/terraform-aws-ecs-web-app

what Update aws provider version requirements why 3.0 AWS provider is already here and we should be able to use the module with it

1
    keyboard_arrow_up