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

fixes typo/reference in README.md #37

Closed
wants to merge 1 commit into from
Closed

fixes typo/reference in README.md #37

wants to merge 1 commit into from

Conversation

camilosantana
Copy link

README references eg-prod-bastion-public resource that doesn't exist yet in the step where it's mentioned. i believe eg-prod-bastion-label is intended here.

Signed-off-by: camilo santana camilo.santana@procore.com

Signed-off-by: camilo santana <camilo.santana@procore.com>
@camilosantana camilosantana changed the title fixes typo/reference fixes typo/reference in README.md Aug 23, 2018
@@ -52,7 +52,7 @@ module "eg_prod_bastion_label" {
}
```

This will create an `id` with the value of `eg-prod-bastion-public`.
This will create an `id` with the value of `eg_prod_bastion_label`.
Copy link
Member

Choose a reason for hiding this comment

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

Hrm... so I think what we're trying to say is that the value of eg_prod_bastion_label.id is eg-prod-bastion-public because namespace=eg, stage=prod, name=bastion and attributes=public.

Perhaps there's a better way to say this or explain it?

Copy link
Author

Choose a reason for hiding this comment

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

i think that's an excellent explanation.

Perhaps,
"this will generate an id with the value of eg-prod-bastion-public based off the key:value pairs above."

or any version thereof. my point is that line would benefit from further elaboration.

thanks for clarifying.

@osterman osterman requested a review from aknysh August 23, 2018 19:16
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

Addressed in #36

@aknysh
Copy link
Member

aknysh commented Aug 24, 2018

Addressed in #36

@aknysh aknysh closed this Aug 24, 2018
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