-
Notifications
You must be signed in to change notification settings - Fork 10
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
refactor(module/test_infrastructure): module creation and adjusted examples #10
Conversation
/plan paths="modules/vmseries"
|
/plan paths="modules/vmseries"
|
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.
We need to update the README files regarding the following comments:
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.
Generally, it looks good , I have only 2 optional comments.
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.
Overall great job. I have few comments, but they are optional 👍
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.
That's great work in deed 👍🏻 I just have one comment, I think we don't handle the auto creation of hidden readme files with a dot for .README.md
, this can result us forgetting about it and committing a new README.md
file without "." in the test_instrastructure
module.
I think we can handle this in one of the 2 ways;
- Creating a terraform-docs yaml file within the test_infrastructure module itself so it has the output set as
.README.md
. This config should take precedence over the one in the root module if I understand correctly. We might need to remove yaml config file reference in pre-commit hooks. Need to be tested.
https://terraform-docs.io/user-guide/configuration/ - Targeting the test_infrastructure module separately in pre-commit hooks, by creating another hook with specific
--output-file
parameter to generate.README.md
in the module. Need to be tested as well, might have a conflict when we have a generic terraform-docs and module specific terraform-docs hook at the same time.
https://terraform-docs.io/how-to/pre-commit-hooks/
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.
Opened a separate case for the .README.md
file handling referred to here: #10 (review)
Description
PR delivers changes for test_infrastructure:
test_infrastructure
module and integrating it with reference architecture examples in favour of a separate test infra exampleMotivation and Context
#8
How Has This Been Tested?
Refactored
common_vmseries
example has been deployed in the lab.Types of changes
Checklist