-
Notifications
You must be signed in to change notification settings - Fork 913
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
feat: Enable populating of data directory and .sample files and update dependencies in 0-cicd-github #1240
Conversation
…ndencies The Readme.md files reference the data directory and .sample files, but the code did not allow for their populating. This update enables the copying of the data directory and .sample files, with the data directory being populating as a data.sample directory to prevent overwriting any existing data directory. Additionally, dependencies have been updated by adding the depends_on section to several resources to ensure that the dependencies are in the correct order. This update addresses some states that were not being handled previously. There is a minor known issue with Pull Request creation in the current state of the code. The Pull Request is only created after the first run has occurred. A fix for this issue is currently being worked on and will be addressed in a separate Pull Request. However, this issue does not affect the main functionality of the code.
Hey Anton, I'm not entirely sure we want to copy sample files: in my opinion once you have CI/CD set up you don't need samples and have the correct files in place. |
I agree with your opinion. I also had mixed feelings about the idea. From my own experience, it took me several hours to deduce the functionality of the modules and variables from the source code, including the structure and contents of the data directory. Similarly, I did not expect to find terraform.tfvars.sample in the original repo. Having copies of these files in the current repo would have saved me a lot of time as I was reading the Readme.md file and getting acquainted with the provisioned repo. Perhaps we could add a note in the Readme.md files to indicate the location of such files in the original repo. Pros:
Cons:
I will, of course, accept your decision. Could you please let me know what the decision is? |
I see your points, and also that we have two different approaches to CI/CD. What I and most of my colleagues do, is to initially apply the stages manually a few times until everything is working well, the custom tfvars have the desired values, etc. then transfer the files to CI/CD, at which point you already have tfvars and factory data has been customized. For our approach, moving tfvars and data potentially creates a problem, or at least adds non-relevant files to the repo. Why don't we do both the things you suggested, with twist? We add a notice to the READMEs, and we also implement this change but we gate it behind a boolean variable, possibly defaulting to false? And we explain in the cicd github stage that it exists, and should be used when CI/CD is used right from the start. Would this work for you? |
@ludoo I would like to check the proposal. Which version would be better? Simple: repositories = {
gcp_fast_00_bootstrap = {
create_options = {
description = "FAST bootstrap."
}
features = {
issues = true
}
populate_from = "../../stages/0-bootstrap"
populate_samples = true
}
# tftest skip More future-proof: repositories = {
gcp_fast_00_bootstrap = {
create_options = {
description = "FAST bootstrap."
}
features = {
issues = true
}
populate_from = "../../stages/0-bootstrap"
populate_options = {
populate_samples = true
}
}
# tftest skip |
I prefer the first one, but either works fine if you have a strong preference :) |
…-of-data-directory-sample-files-and-update-dependencies
…-of-data-directory-sample-files-and-update-dependencies
…-of-data-directory-sample-files-and-update-dependencies
check your mail, you should then be able to merge yourself |
…-of-data-directory-sample-files-and-update-dependencies
The Readme.md files reference the data directory and .sample files, but the code did not allow for their populating. This update enables the copying of the data directory and .sample files, with the data directory being populating as a data.sample directory to prevent overwriting any existing data directory.
Additionally, dependencies have been updated by adding the depends_on section to several resources to ensure that the dependencies are in the correct order. This update addresses some states that were not being handled previously.
There is a minor known issue with Pull Request creation in the current state of the code. The Pull Request is only created after the first run has occurred. A fix for this issue is currently being worked on and will be addressed in a separate Pull Request. However, this issue does not affect the main functionality of the code.
Please review the changes and suggest any possible improvements or approve the pull request.