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 force_destroy option to bigquery dataset #2986

Merged
merged 6 commits into from
Feb 8, 2019

Conversation

zhilingc
Copy link
Contributor

@zhilingc zhilingc commented Feb 2, 2019

Added the force_destroy option for bigquery datasets, as described in #2050.

@ghost ghost added the size/xs label Feb 2, 2019
@rileykarson rileykarson self-requested a review February 2, 2019 01:55
Copy link
Collaborator

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

Just a few small things, I'm wondering what the field should be named (see inline) and we'll need to document the field + test it.

For testing, you can add it to the end of this test case if you'd like: https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_bigquery_dataset_test.go#L183

In terms of documentation, the description of the parameter in the official docs is pretty good, so we can copy that over.

google/resource_bigquery_dataset.go Outdated Show resolved Hide resolved
@ghost ghost added size/l and removed size/xs labels Feb 2, 2019
@zhilingc
Copy link
Contributor Author

zhilingc commented Feb 3, 2019

@rileykarson Added the documentation and the tests. I've excluded the field from the ImportState verification step because I don't think it's part of the import state at all, let me know if this is an incorrect assumption to make.

Copy link
Collaborator

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Looking great! Just one more comment & a couple spacing nitpicks.

google/resource_bigquery_dataset_test.go Outdated Show resolved Hide resolved
google/resource_bigquery_dataset_test.go Outdated Show resolved Hide resolved
google/resource_bigquery_dataset.go Outdated Show resolved Hide resolved
@ghost ghost added size/m and removed size/l labels Feb 4, 2019
@zhilingc
Copy link
Contributor Author

zhilingc commented Feb 4, 2019

Looking great! Just one more comment & a couple spacing nitpicks.

Done. Thanks so much for the quick reviews!

Copy link
Collaborator

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution!

All that's left is on my end- I'm going to duplicate this up in to our shared code generator, Magic Modules, in order to ensure that this change gets added to google-beta as well. Once I've done so, I'll merge this PR & your feature will be available in the next provider release.

@zhilingc
Copy link
Contributor Author

zhilingc commented Feb 5, 2019

awesome 👍

@ghost
Copy link

ghost commented Mar 11, 2019

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!

@ghost ghost locked and limited conversation to collaborators Mar 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants