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

Add il-central-1 region #31

Merged
merged 6 commits into from Jan 9, 2024
Merged

Add il-central-1 region #31

merged 6 commits into from Jan 9, 2024

Conversation

jasonmk
Copy link
Contributor

@jasonmk jasonmk commented Oct 31, 2023

what

Add new Tel Aviv (il-central-1) region

why

Provide full coverage

references

@jasonmk jasonmk requested review from a team as code owners October 31, 2023 16:29
aknysh
aknysh reviewed Jan 8, 2024
View reviewed changes
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.

@jasonmk thanks for the PR, looks good

can you please update https://github.com/cloudposse/terraform-aws-utils/blob/main/LICENSE#L189

Copyright 2020-2024 Cloud Posse, LLC

and run the following commands and commit the changes

make init
make github/init
make readme

@aknysh
Copy link
Member

aknysh commented Jan 8, 2024

@jasonmk please also format all TF code, see https://github.com/cloudposse/terraform-aws-utils/actions/runs/7452811313/job/20276897900?pr=31

@aknysh
Copy link
Member

aknysh commented Jan 8, 2024

/terratest

@aknysh
Copy link
Member

aknysh commented Jan 8, 2024

@jasonmk one more thing :)

please increase the test timeout from 5m to 20m in
https://github.com/cloudposse/terraform-aws-utils/blob/main/test/src/Makefile#L18

panic: test timed out after 5m0s
running tests:
	TestExamplesComplete (5m0s)
``

@aknysh
Copy link
Member

aknysh commented Jan 8, 2024

/terratest

@aknysh
Copy link
Member

aknysh commented Jan 8, 2024

@jasonmk thanks again :)
I'll have to ask you to add ca-west-1 region and all the abbreviations

https://github.com/cloudposse/actions/actions/runs/7453067798/job/20277664634

       	Test:       	TestExamplesComplete
        	Messages:   	Abbreviation map identity is missing entry for region ca-west-1

ca-west-1 was added by AWS after this module was last tested.

Thank you very much

@aknysh
Copy link
Member

aknysh commented Jan 9, 2024

/terratest

aknysh
aknysh approved these changes Jan 9, 2024
View reviewed changes
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 @jasonmk

@aknysh aknysh merged commit c86b8d6 into cloudposse:main Jan 9, 2024
9 checks passed
@jasonmk jasonmk deleted the add-il-central-1-region branch January 9, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknysh aknysh aknysh approved these changes

@nitrocode nitrocode Awaiting requested review from nitrocode nitrocode is a code owner automatically assigned from cloudposse/contributors

@woz5999 woz5999 Awaiting requested review from woz5999 woz5999 is a code owner automatically assigned from cloudposse/contributors

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jasonmk @aknysh