-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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 tags support to aws_nat_gateway resource #1625
Conversation
Acceptance test:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ewbankkit
Thanks for the work here! 👍
Just left 2 small comments of really small importance.
Tell me what you think!
Thanks :)
aws/resource_aws_nat_gateway.go
Outdated
|
||
if err := setTags(conn, d); err != nil { | ||
return err | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else is not needed since the if returns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
aws/resource_aws_nat_gateway_test.go
Outdated
|
||
const testAccNatGatewayConfigTags = ` | ||
resource "aws_vpc" "vpc" { | ||
cidr_block = "10.0.0.0/16" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitpick: can you fix the indentation through the tests here? perhaps using 2 spaces, so that it is consistent with other ones?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ewbankkit
This looks very good to me!
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSNatGateway_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSNatGateway_ -timeout 120m
=== RUN TestAccAWSNatGateway_importBasic
--- PASS: TestAccAWSNatGateway_importBasic (212.37s)
=== RUN TestAccAWSNatGateway_basic
--- PASS: TestAccAWSNatGateway_basic (190.04s)
=== RUN TestAccAWSNatGateway_tags
--- PASS: TestAccAWSNatGateway_tags (225.70s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 628.147s
Thanks a lot for your work! 👍 :)
Add tags support to aws_nat_gateway resource
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Fixes #1624.