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

update to context #24

Merged
merged 5 commits into from
Oct 22, 2020
Merged

update to context #24

merged 5 commits into from
Oct 22, 2020

Conversation

syphernl
Copy link
Contributor

what

  • Update to context.tf

why

  • Standardization and interoperability

@syphernl syphernl requested a review from a team as a code owner October 19, 2020 07:40
@syphernl syphernl requested review from Makeshift and RothAndrew and removed request for a team October 19, 2020 07:40
@syphernl
Copy link
Contributor Author

I would really appreciate it if this PR could be tagged with hacktoberfest-accepted so it counts towards my Hacktoberfest PR's (since the repo isn't tagged with hacktoberfest)

Copy link

@RothAndrew RothAndrew left a comment

Choose a reason for hiding this comment

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

Referring review to leadership, @osterman or other please take a look. I don't know enough about context.tf to know if this was done the way we want it to be done.

For the record, I effing hate hacktoberfest. It has been directly responsible for an absolute tidal wave of crap that has inundated many open source repos with low quality PRs.

Not saying this is one of those, just voicing a general opinion 😄

@RothAndrew RothAndrew requested a review from osterman October 19, 2020 16:27
@Gowiem
Copy link
Member

Gowiem commented Oct 19, 2020

/test all

@osterman osterman requested a review from aknysh October 19, 2020 18:50
@osterman
Copy link
Member

Thanks @syphernl (Frank!) We'll have @aknysh take a look

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.

please see comments

examples/complete/variables.tf Show resolved Hide resolved
Copy link

@RothAndrew RothAndrew left a comment

Choose a reason for hiding this comment

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

Removing my "request changes" as @aknysh is now reviewing

@RothAndrew
Copy link

/test all

1 similar comment
@aknysh
Copy link
Member

aknysh commented Oct 20, 2020

/test all

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.

@syphernl syphernl requested a review from aknysh October 21, 2020 06:32
@aknysh
Copy link
Member

aknysh commented Oct 21, 2020

/test all

@aknysh
Copy link
Member

aknysh commented Oct 21, 2020

@syphernl
please fix these errors:

estExamplesComplete 2020-10-21T14:37:38Z command.go:121:   on ../../public.tf line 22, in resource "aws_subnet" "public":
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:   22:     "Name"      = "${module.public_label.id}${var.delimiter}${element(var.subnet_names, count.index)}"
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:     |----------------
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:     | var.delimiter is null
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: 
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: The expression result is null. Cannot include a null value in a string
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: template.
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: 
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: 
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: Error: Invalid template interpolation value
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121: 
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:   on ../../public.tf line 22, in resource "aws_subnet" "public":
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:   22:     "Name"      = "${module.public_label.id}${var.delimiter}${element(var.subnet_names, count.index)}"
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:     |----------------
TestExamplesComplete 2020-10-21T14:37:38Z command.go:121:     | var.delimiter is null

All variables that are present now in the context.tf file, need to be replaced with module.this.xxxx.
For example, var.delimiter needs to be replaced with module.this.delimeter in all files and examples.
The same for all other variables.

What you can do to speed up the testing and find all errors, is:

  1. cd examples/complete
  2. terraform init
  3. terraform plan

It will show you if there are any errors before we deploy the example on AWS

Thank you for all your work on PRs.

@syphernl
Copy link
Contributor Author

@aknysh The delimiter has been fixed and the sample is passing terraform plan -var-file=fixtures.us-east-2.tfvars

@aknysh
Copy link
Member

aknysh commented Oct 22, 2020

/test all

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.

thanks @syphernl
It looks good and works now

@aknysh aknysh merged commit 78f825b into cloudposse:master Oct 22, 2020
@syphernl syphernl deleted the feat/context branch October 22, 2020 18:12
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.

5 participants