Skip to content
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 custom broker name #70

Closed
wants to merge 11 commits into from
Closed

Conversation

joshuabalduff
Copy link
Contributor

what

Allow a user to set broker_name to a custom name.

why

Flexibility

references

@joshuabalduff
Copy link
Contributor Author

@dotCipher I ran make readme but seems it removed some things, not sure if that is intended or not.

.terraform.lock.hcl Outdated Show resolved Hide resolved
variables.tf Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, please see a few comments

examples/complete/main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshuabalduff thank you for all the changes, please remove the file .terraform.lock.hcl - we don't commit TF state lock files

@aknysh
Copy link
Member

aknysh commented Dec 4, 2023

@joshuabalduff sorry to bother you again, the PR looks good now, please run the following commands and commit the changes:

make init
make github/init
make readme

@joshuabalduff
Copy link
Contributor Author

No, you are totally fine. It's been a long weekend with code changes and trying to keep up. Thanks for the help. 😄

@aknysh
Copy link
Member

aknysh commented Dec 4, 2023

/terratest

@aknysh
Copy link
Member

aknysh commented Dec 5, 2023

@joshuabalduff
because of these errors https://github.com/cloudposse/actions/actions/runs/7091589923/job/19301058052,
I have to ask you to update the module versions in the example https://github.com/cloudposse/terraform-aws-eks-cluster/blob/main/examples/complete/main.tf

In particular:

module "vpc" - please use the version AND the input names as defined here https://github.com/cloudposse/terraform-aws-eks-cluster/blob/main/examples/complete/main.tf#L30

module "subnets" - please use the version AND the input names as defined here https://github.com/cloudposse/terraform-aws-eks-cluster/blob/main/examples/complete/main.tf#L40

thanks

@hans-d hans-d added the stale This PR has gone stale label Mar 2, 2024
@hans-d
Copy link
Contributor

hans-d commented Mar 3, 2024

@joshuabalduff Hi, can you update the pr so that it passes the tests? otherwise, it is likely to be closed due to staleness.

Copy link

mergify bot commented Mar 9, 2024

Thanks @joshuabalduff for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

Copy link

mergify bot commented Mar 9, 2024

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@mergify mergify bot added the needs-cloudposse Needs Cloud Posse assistance label Mar 9, 2024
Copy link

mergify bot commented Mar 9, 2024

This pull request now has conflicts. Could you fix it @joshuabalduff? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Mar 9, 2024
@mergify mergify bot closed this Mar 9, 2024
Copy link

mergify bot commented Mar 9, 2024

This PR has been closed due to inactivity and merge conflicts.
Please resolve the conflicts and reopen if necessary.

@mergify mergify bot removed conflict This PR has conflicts needs-cloudposse Needs Cloud Posse assistance labels Mar 9, 2024
@mergify mergify bot removed the stale This PR has gone stale label Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants