-
-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Terraform cloudposse/ecs-codepipeline/aws to v0.33.0 #225
Update Terraform cloudposse/ecs-codepipeline/aws to v0.33.0 #225
Conversation
@Gowiem @korenyoni - is it possible to get this reviewed or landed? Would be helpful to have this in |
Pulling in @joe-niland since he's an expert on this module. Joe, figured you may have an opinion here. |
/test all |
@adamantike @getglad -- This looks like a solid contribution, thanks! I'm checking with the rest of the maintainer team on if this should be a major version rev or not. I believe it should, but semver is followed differently everywhere and it's we've just started rev'ing past v0.x.y, so it's good to check. Will follow up when we can move this forward 👍 |
Thanks for the ping @Gowiem. Since this introduces a breaking change, I think we need to include information about how to change a consuming module in the PR description (example). As far as I can see, it's just:
|
@joe-niland good call on the breaking changes info and including that in the PR description. @adamantike do you mind providing that info into your description and then we'll get this merged and cut a release? Here is a great PR example of what that should look like: cloudposse/terraform-aws-s3-bucket#158 |
Looking forward to get this in which would fix the below on Terraform 1.4.x on arm64 macOS.
|
@max-lobur @Gowiem @joe-niland friendly re-bump on this. It sounds like this was down to documentation on the PR description, but now seeing some failures around a pinning to Any ideas on how we can get it unblocked and merged? |
@getglad we (@max-lobur specifically) just did a sweeping change to our automation to add tflint, so things like that are sadly a necessary evil that we need to deal with on open PRs. We'll need @adamantike to provide the PR description updates that are discussed above and fix the tflint issues you mentioned. @adamantike friendly ping on this. Mind updating the above? Alternatively if you're looking to get this merged @getglad, feel free to fork this PR's branch, make the necessary updates, put up another PR, and request me as a reviewer. We'll get this moved along quickly in that case. |
d1467e4
to
2eec517
Compare
Sorry for not tackling this before. I have rebased my changes, and updated the PR description to include the information about the breaking change and how to upgrade to the new minor version! |
/terratest |
@adamantike sorry to continue to drag this out, but we've got some lint issues and terraform failures due to the |
It's probably the same issue I needed to fix in the complete example, because of very old versions of |
2eec517
to
9298902
Compare
/terratest |
@adamantike can you bump this line to v1.3+? https://github.com/cloudposse/terraform-aws-ecs-web-app/blob/main/examples/complete/versions.tf#L2 |
|
/terratest |
@adamantike hi, can you update this pr so that all tests pass? otherwise this PR will be closed due to staleness. thanks! |
/terratest |
@hans-d this is a quite important PR as its the only blocker for M1 compatibility of this module. I’m taking it over to see if I can get tests passing |
/terratest |
/terratest |
…om/adamantike/terraform-aws-ecs-web-app into misc/bump-ecs_codepipeline-v0.32.0
/terratest |
Breaking changes
This release introduces a breaking change that affects projects using the
github_webhooks_token
variable.Since Terraform 1.2.0, modules to be used with the
for_each
,count
, anddepends_on
arguments must not contain their own provider configurations. Thegithub_webhooks_token
variable was being used to instantiate agithub
provider, which must now be instantiated and provided separately.If your project is affected by this change, the following steps will allow you to upgrade to this version:
Add the GitHub provider to your project:
Instantiate the provider (feel free to use any other supported Authentication mechanism):
Stop providing the
github_webhooks_token
variable to this module.what
cloudposse/ecs-codepipeline/aws
module.why
cloudposse/repository-webhooks/github
module, which removed the hardcodedgithub
provider, so some variables have been removed in favor of instantiating the provider separately.references