-
Notifications
You must be signed in to change notification settings - Fork 1
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
Templatize via Cookiecutter for enhanced audience utility #10
Conversation
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.
I can see the value in using cookiecutter to customize the names of files and strings in documentation. In the case of Terraform config, I'd personally opt for one of Terraform's existing methods for setting input variables (https://developer.hashicorp.com/terraform/language/values/variables#assigning-values-to-root-module-variables) rather than writing them into the default value fields within your modules. Typically, you'd create a terraform.tfvars
file in the folder where you expect your users to invoke Terraform and instruct them to modify it; you could also populate it with cookiecutter.
Co-Authored-By: Faisal Alquaddoomi <faisal.alquaddoomi@cuanschutz.edu>
This reverts commit 855b6a4.
Co-Authored-By: Faisal Alquaddoomi <faisal.alquaddoomi@cuanschutz.edu>
Thank you @falquaddoomi ! I've added a |
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.
Reviewed writing, LGTM
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.
Oof, sorry, forgot to submit this review. There's just the one thing about the .tfvars
file needing to be renamed to something TF will automatically use, but otherwise this all looks great!
…rm.tfvars. Updated links to that file in the state mangement, infrastructure sections.
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.
Looks good! Thanks for taking my previous suggestions into consideration.
Thank you @falquaddoomi and @vincerubinetti for your reviews! Merging this in now. |
This PR addresses #4 by moving to a Cookiecutter implementation for this repository. The benefit of this change entails making this template more accessible through Cookiecutter commands and more efficient through common text replacements which would otherwise need to be performed manually.
Note: After this is merged I plan to remove the Github template status (which will no longer be as helpful or accurate to the intent).
A demonstration of the audience experience after a merge can be found from the following command:
Thanks in advance for any thoughts and comments you may have!
Closes #4