#pr-reviews (2024-04)
Pull Request Reviews for Cloud Posse Projects
2024-04-02
Simple PR to do some type enforcement on null label: https://github.com/cloudposse/terraform-null-label/pull/158
Ran into too many people using enabled = "false" instead of enabled = false and this will fix that.
@Jeremy G (Cloud Posse) to chime in here
@Chris Dobbyn I don’t mean to be harsh, but I don’t know how to say this more politely: AFAIK, the problem you are trying to solve does not exist, and if it did, your PR would not address it. I put more details in my comment closing your PR.
In examples/complete/label5.tf we set enabled = false. You can try different values and see for yourself that enabled = "false" sets enabled to the boolean value false, not true.
So the key takeaway here is that:
even if
enabled = "false"resulted invar.enabled == trueneither this validation, nor any validation, would be able to detect and correct that situation, because the validation runs after the conversion takes place.
2024-04-10
Please review https://github.com/cloudposse/terraform-aws-dynamodb/pull/126 to override the dynamodb table name without having to override the context inputs
2024-04-11
Can someone approve the GitHub workflow on https://github.com/cloudposse/terraform-aws-cloudtrail/pull/70
the test failed again, my guess is that there is more input needed for the updated module for the examples to pass
I would put money on that these tests would fail on the main branch.
the test must pass for us to be able to merge
I can’t see that the tests have ever been run on a merged PR in this repo.
I do not know how long they are retained
@jose.amengual, I’ve bumped the cloudtrail-s3-bucket module version on this PR. Can you rerun the tests, which should pass now.
2024-04-12
Can someone approve the GitHub workflow on https://github.com/cloudposse/terraform-aws-cloudtrail-s3-bucket/pull/91
And run the tests?