[go: up one dir, main page]

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

Adding 3rd example to repo #12

Closed
wants to merge 5 commits into from
Closed

Adding 3rd example to repo #12

wants to merge 5 commits into from

Conversation

vtimd
Copy link
Contributor
@vtimd vtimd commented Dec 4, 2023

This pull request will add the 3rd set of example code to the repo for the blog on how to use terraform outputs across stacks.

@vtimd vtimd requested review from a team as code owners December 4, 2023 19:27
Copy link
Contributor
@soerenmartius soerenmartius left a comment

Choose a reason for hiding this comment

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

Please see the comments. Also too, did you test this code, does it actually run? (does it a) compile with terramate generate and b) work with terraform apply ?

"dependencies" = [
"stacks/vpc",
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should always add a "newline at the end of each file", hence the stop sign here.

Here's how to do that with vscode: https://stackoverflow.com/questions/44704968/visual-studio-code-insert-newline-at-the-end-of-files

And that's why: https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline

03-terraform-outputs-across-stacks/stacks/EC2/main.tf Outdated Show resolved Hide resolved
@soerenmartius
Copy link
Contributor

@vtimd just tried the code locally, it unfortunately doesn't work (try run terramate generate to trigger the code generation)
Screenshot 2023-12-04 at 22 40 10

Keep in mind that the example we defined in last weeks session was never tested and was meant as an example for you how the final implementation could look like

@vtimd
Copy link
Contributor Author
vtimd commented Dec 5, 2023

This PR was just a placeholder for the file links. I didn’t test the code. I’m going to get it fixed in parallel while I write the blog. I just needed the GitHub links for the blog. I knew the code was wrong when I saved it in Code. It was all red 😂

@soerenmartius
Copy link
Contributor

This PR was just a placeholder for the file links. I didn’t test the code. I’m going to get it fixed in parallel while I write the blog. I just needed the GitHub links for the blog. I knew the code was wrong when I saved it in Code. It was all red 😂

Ok understood. In general it's a recommended best practice to open PRs that aren't ready for review yet as draft pull requests as otherwise folks receive a notification that your PR is ready for review

vtimd and others added 3 commits December 22, 2023 09:52
Co-authored-by: Kyle Knight <106227170+kkmlr@users.noreply.github.com>
Co-authored-by: Kyle Knight <106227170+kkmlr@users.noreply.github.com>
Co-authored-by: Sören Martius <soeren.martius@gmail.com>
Co-authored-by: Kyle Knight <106227170+kkmlr@users.noreply.github.com>
Copy link
Contributor
@soerenmartius soerenmartius left a comment

Choose a reason for hiding this comment

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

left more comments in shortcut

region = "us-east-1"
encrypt = true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting issue (run terramate fmt)

@vtimd
Copy link
Contributor Author
vtimd commented Jan 2, 2024

Closing. Starting Fresh.

@vtimd vtimd closed this Jan 2, 2024
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