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

Add support for Terraform v0.12 #5 #6

Merged
merged 1 commit into from
Jul 26, 2019

Conversation

ingwarr
Copy link
Contributor

@ingwarr ingwarr commented Jul 2, 2019

Fixes #5

@aaron-lane aaron-lane added the enhancement New feature or request label Jul 2, 2019
Copy link

@Jberlinsky Jberlinsky left a comment

Choose a reason for hiding this comment

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

Tests fail on localhost as provided. The following additional change needs to be made:

  • In Makefile, the DOCKER_TAG_BASE_KITCHEN_TERRAFORM variable must be updated to 2.0.0 so that the tests run against Terraform 0.12.x.

In general, please make sure that module tests are run before pull requests are submitted. I've created #7 to ensure that we get CI set up in a timely manner for this module.

Copy link

@Jberlinsky Jberlinsky left a comment

Choose a reason for hiding this comment

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

  • CHANGELOG needs to be updated
  • README needs to be updated
  • Linting needs to be run

outputs.tf Show resolved Hide resolved
@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 8, 2019

I run tests without docker before the push

@Jberlinsky
Copy link

@ingwarr While I appreciate that, the contribution guidelines linked to in the README specifically encourage running tests in Docker. We should not merge a pull request with broken tests when executing in the documentation-suggested manner.

@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 10, 2019

@Jberlinsky Re-test Done

outputs.tf Show resolved Hide resolved
@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 16, 2019

This still doesn't look like terraform fmt or our linting make task was run.
@Jberlinsky I run terraform fmt without any results even if I manually added additional spaces, terraform fmt undo my changes.

@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 22, 2019

image
image

@aaron-lane aaron-lane self-requested a review July 22, 2019 20:26
@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 22, 2019

@Jberlinsky command find . -type f -name "*.tf" -exec terraform fmt {} \; haven't brought any result

CHANGELOG.md Show resolved Hide resolved
Makefile Show resolved Hide resolved
main.tf Show resolved Hide resolved
@ingwarr
Copy link
Contributor Author

ingwarr commented Jul 22, 2019

image

Copy link

@bohdanyurov-gl bohdanyurov-gl left a comment

Choose a reason for hiding this comment

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

Please squash all commits and add screenshots for tests after applying changes.

CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
outputs.tf Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@aaron-lane aaron-lane merged commit 55fb3f0 into terraform-google-modules:master Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Terraform v0.12
4 participants