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.

Sign up for GitHub

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

Jump to bottom

Adds context and example/complete #17

Merged
merged 7 commits into from
Jan 18, 2021
Merged

Adds context and example/complete #17

merged 7 commits into from
Jan 18, 2021

Conversation

lezavala
Copy link
Contributor

what

  • Adds context.tf
  • Adds example/complete.

why

  • Attempt to add a decent test and bring repo up to standards.

@lezavala lezavala requested a review from a team as a code owner January 16, 2021 01:10
@lezavala lezavala requested review from SweetOps and brcnblc and removed request for a team January 16, 2021 01:10
@aknysh
Copy link
Member

aknysh commented Jan 16, 2021

/rebuild-readme

@aknysh
Copy link
Member

aknysh commented Jan 16, 2021

/terraform-fmt

@aknysh
Copy link
Member

aknysh commented Jan 16, 2021

/test all

@aknysh
Copy link
Member

aknysh commented Jan 16, 2021 •
edited
Loading

@lezavala thanks for the PR.
Could you please run the following commands:

make init
make github/init
make readme

This will bring in all the latest GitHub actions we are adding to all modules.

Please also update https://github.com/cloudposse/terraform-aws-sns-lambda-notify-slack/blob/master/versions.tf to similar to what you did for examples/complete/versions.tf to make the module work with TF 0.13 and 0.14

Thank you

@lezavala lezavala requested a review from a team as a code owner January 18, 2021 16:12
@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

/test all

@aknysh aknysh added the terraform/0.13 Module requires Terraform 0.13 or later label Jan 18, 2021
@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

/rebuild-readme

@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

/test all

@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

@lezavala
thanks a lot for the updates.
Everything looks good now, except for some reason README was not updated.
Also, TF code needs to be formatted in the module and in the example.

We usually execute ChatOps commands as comments on PRs to rebuild README and format TF code, but it looks like our bot does not have access to your repo (there is some protection).

Can you please execute these commands again:

make init
make readme

and also terraform fmt on the module and on the example.

Thank you

@lezavala lezavala closed this Jan 18, 2021
@lezavala
Copy link
Contributor Author

Accidentally closed this

@lezavala lezavala reopened this Jan 18, 2021
@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

/test all

@aknysh aknysh merged commit 2c16108 into cloudposse:tf013-fixes Jan 18, 2021
aknysh pushed a commit that referenced this pull request Jan 18, 2021
@osterman @lezavala
Additional TF 0.13 Fixes (#15)
ea0674b 
* Support terraform 0.13

* Adds context and example/complete (#17)

* Having a go at the examples dir.

* Fix the duplicate var declaration in test.

* Adds context and fixes test

* Add GitHub actions, and updates readme

* Updates version.tf

* Updates readme

* Format

Co-authored-by: Leo <38137291+lezavala@users.noreply.github.com>
@aknysh
Copy link
Member

aknysh commented Jan 18, 2021

@lezavala
new release with all your changes https://github.com/cloudposse/terraform-aws-sns-lambda-notify-slack/releases/tag/0.5.0

@lezavala lezavala deleted the addTest branch January 18, 2021 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SweetOps SweetOps Awaiting requested review from SweetOps

@brcnblc brcnblc Awaiting requested review from brcnblc

Assignees
No one assigned
Labels
terraform/0.13 Module requires Terraform 0.13 or later
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lezavala @aknysh