5 messages
Pull Request Reviews for Cloud Posse Projects
kevcubealmost 4 years ago
requesting review on this one (not my PR) https://github.com/cloudposse/terraform-aws-rds-cluster/pull/138
it’d be helpful to have
it’d be helpful to have
azecalmost 4 years ago
I was watching recording of this week’s demo (I missed live) and got interested about atmos.
Reading through https://github.com/cloudposse/atmos#centralized-project-configuration , I realized that refs to the config files in the example dir structure are little bit outdated.
Do these changes require GH Issue to be opened or PR directly is ok?
Reading through https://github.com/cloudposse/atmos#centralized-project-configuration , I realized that refs to the config files in the example dir structure are little bit outdated.
Do these changes require GH Issue to be opened or PR directly is ok?
C
Chris Dobbynover 3 years ago
I don't have a PR yet, but I'm hoping I could gather some thoughts/feedback before I go too far. I was looking at the module aws-vpc-peering. The problem I would like to try to solve is where you have multiple
Would you:
• Fork at this point to preserve backwards compatibility
• Create an option that determines which is used
• Completely replace the count with a for_each
• Bite the bullet, accept the count / index stuff and just make it work with what's there
My personal issue with the count is when things change multiple routes need to update (because indexes might also change). Using a for_each is safer because you can create a key that identifies the elements involved and prevent the mass re-creation of objects.
associated_cidr_blocks to a vpc but you only want to route a subset of them. I have written code that will do this via a for_each, but adding this logic into the count and index magic that's happening is a little painful.Would you:
• Fork at this point to preserve backwards compatibility
• Create an option that determines which is used
• Completely replace the count with a for_each
• Bite the bullet, accept the count / index stuff and just make it work with what's there
My personal issue with the count is when things change multiple routes need to update (because indexes might also change). Using a for_each is safer because you can create a key that identifies the elements involved and prevent the mass re-creation of objects.
Mover 3 years ago
an older PR of mine to review https://github.com/cloudposse/terraform-aws-rds/pull/136
then we can have v1.0.0 of this module
then we can have v1.0.0 of this module