-
-
Notifications
You must be signed in to change notification settings - Fork 133
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
Allow to use ECR replication #103
Conversation
can someone run tests?🙂 |
@dmitrijn can you please address the linting issues in your new example as well? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments for requested changes -- Thanks!
@dmitrijn friendly ping on this -- want to wrap this up with the requested changes and we'll get it merged? |
@Gowiem please run tests. |
@dmitrijn thank you for the PR make init
make github/init
make readme |
variables.tf
Outdated
|
||
variable "replication_configuration_rules" { | ||
description = "The replication rules for a replication configuration. A maximum of 10 are allowed per" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update the description
A maximum of 10 are allowed per <.....>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please see comments
@dmitrijn this one is close, can you make the remaining change so this can be merged? |
This pull request is now in conflict. Could you fix it @dmitrijn? 🙏 |
@hans-d I took this one over, can you review? |
/terratest |
@Gowiem Can you check if all is ok for you as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Closes #99