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

Commenting out code for creating 2nd interface on VMs. Not needed. Ch… #361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dojacobs
Copy link
Contributor

…anging machine to t2.micro to save money.

Copy link
Contributor

@neelimamukiri neelimamukiri left a comment

Choose a reason for hiding this comment

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

LGTM. Can we may be split this into two - aws.tf and aws-large.tf or something similar, instead of having commented out and stale code?

@dojacobs
Copy link
Contributor Author

Since this is only used by Jenkins, I don't see a point in having a "small" and "large" version. If you would like, I could make machine type an external variable that you specify in the aws.tfvars file or it could be specified on the command line when you do the terraform apply. Otherwise it's just one line inside the script to change the machine type that is used.

@neelimamukiri
Copy link
Contributor

@dojacobs, in that case why not just delete the commented lines?

@dojacobs
Copy link
Contributor Author

@rhim's thinking was that we might need them in the future. It's no big deal either way. What is the normal process?

@dojacobs
Copy link
Contributor Author

Updated file with the commented lines deleted.

@dseevr
Copy link
Contributor

dseevr commented Aug 15, 2017

@contiv/maintainers-deployment PTAL

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