Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Move IaC files to a specified folder #144

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

eranelbaz
Copy link
Contributor

Move all the IaC files from the project root to a sub folder
Fix #118

Signed-off-by: Elbaz <[email protected]>
Signed-off-by: Elbaz <[email protected]>
Signed-off-by: Elbaz <[email protected]>
@@ -44,7 +44,9 @@ jobs:
aws-region: ${{ secrets.AWS_REGION }}

- name: Initialize Terraform
run: terraform init
run: |
cd iac
Copy link
Contributor Author

@eranelbaz eranelbaz Oct 5, 2023

Choose a reason for hiding this comment

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

is that the right way to do it?
if so I'll split it to another step

Copy link

@cparedes cparedes Oct 17, 2023

Choose a reason for hiding this comment

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

FWIW it might actually be better to use a working-directory parameter, like so:

Suggested change
cd iac
run: terraform init
working-directory: "./iac"

@eranelbaz eranelbaz marked this pull request as ready for review October 5, 2023 08:09
@eranelbaz eranelbaz requested a review from Yantrio October 5, 2023 08:09
@Yantrio
Copy link
Member

Yantrio commented Oct 6, 2023

I've aired my opinion on this on the attached issue. I propose we close this.

@eranelbaz
Copy link
Contributor Author

@Yantrio we can rename that folder, but let's push this one forward as it would help reading that project
Usually you don't put logic (iac is logic) in your project root, similar to code which si written in src
IaC is no different and it will really much help people get into and read that project

Copy link
Contributor

@rumenvasilev rumenvasilev left a comment

Choose a reason for hiding this comment

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

Naming is hard. I personally would have called it infra to avoid ambiguity, but tbh that's not really that important. Keeping the root tidy is the main thing of this PR and I fully agree with it. It doesn't break anything so I'm all in favour (although my approval carries no weight in pr merge).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move IaC related files to relevant folder
4 participants