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

Remove unused code from target_listener module #43

Merged
merged 6 commits into from
Apr 16, 2018

Conversation

vandrijevik
Copy link
Contributor

Our code in chef-repo never specifies a value for the variable create_listener, so this variable always gets the default value true in the module’s code.

This PR removes the unused variable, along with all the code that is unused as a result of its removal.

My goal is to simplify the code in this repository related to application load balancers, so that we can create a fully-functiona load balancer with one module declaration, instead of the three that we use today. This is the first step towards that goal.

The diff in the planshot is due to AWS changing their API response, which is why enable_cross_zone_load_balancing is gone as an attribute — per this issue and AWS’s documentation:

With Application Load Balancers, cross-zone load balancing is always enabled.

@vandrijevik vandrijevik requested a review from fedekau April 16, 2018 08:49
@vandrijevik
Copy link
Contributor Author

vandrijevik commented Apr 16, 2018

As proof that this doesn’t change anything for an application load balancer in chef-repo which uses the module, the following diff, which updates usage of this module to the code in this branch, yields an empty plan:

-  source = "github.com/tablexi/terraform_modules?ref=7ae7189//aws/ec2/load_balancers/lb"
+  source = "github.com/tablexi/terraform_modules?ref=d77dddc//aws/ec2/load_balancers/lb"

@vandrijevik vandrijevik merged commit 90f4d0e into master Apr 16, 2018
@vandrijevik vandrijevik deleted the va-remove-unused-code-from-target_listener branch April 16, 2018 14:20
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.

2 participants