-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 new data source: google_dns_managed_zone #268
Conversation
|
||
rs, ok := s.RootModule().Resources[rsFullName] | ||
if !ok { | ||
return fmt.Errorf("can't find %s in state", rsFullName) |
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.
Any reason to have different wording for the two error messages?
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.
Other than that I copied and pasted this snippet from another test. No ;)
Fixed
[the official documentation](https://cloud.google.com/dns/zones/) | ||
and | ||
[API](https://cloud.google.com/dns/api/v1/managedZones). | ||
``` |
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.
This should probably be ```tf or ```hcl
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.
Fixed
|
||
## Argument Reference | ||
|
||
* `name` - (Required) A unique name for the resource, required by GCE. |
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.
GCE isn't relevant here, is it?
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.
Oupps. I meant to clean up these descriptions before opening the PR... Forgot about it ;)
## Argument Reference | ||
|
||
* `name` - (Required) A unique name for the resource, required by GCE. | ||
Changing this forces a new resource to be created. |
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.
Not for a data source
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.
Fixed
* `name` - (Required) A unique name for the resource, required by GCE. | ||
Changing this forces a new resource to be created. | ||
|
||
* `project` (optional) - ID of the project to list available cluster versions for. Should match the project the cluster will be deployed to. |
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.
This looks like it needs to be updated from what this page was copied from
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.
Fixed
|
||
* `dns_name` - The DNS name of this zone, e.g. "terraform.io". | ||
|
||
* `description` - A textual description field. Defaults to 'Managed by Terraform'. |
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.
Defaults aren't really relevant for data sources
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.
Fixed
LGTM, merge at will |
Thank you!
…On Mon, Jul 31, 2017 at 9:27 AM Vincent Roseberry ***@***.***> wrote:
Merged #268
<#268>
.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#268 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZo5Rhc0J_NtVb9WqiQ5M8YHN_1eNvhks5sTgBogaJpZM4OnMCA>
.
|
<!-- This change is generated by MagicModules. --> /cc @drebes
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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Added standalone ACM submodule, embedded acm to examples/simple_zonal. Fixes hashicorp#268
Fixes #239
cc/ @johnsonj @danawillow