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

chore(bigquery): update object table docs #494

Merged

Conversation

nevzheng
Copy link
Contributor

@nevzheng nevzheng commented Sep 7, 2023

Description

Fixes #

Note: If you are not associated with Google, open an issue for discussion before submitting a pull request.

Checklist

Readiness

  • Yes, merge this PR after it is approved
  • No, don't merge this PR after it is approved

Style

Testing

Intended location

API enablement

  • If the sample needs an API enabled to pass testing, I have added the service to the Test setup file

Review

  • If this sample adds a new directory, I have added codeowners to the CODEOWNERS file

@nevzheng nevzheng requested review from a team as code owners September 7, 2023 22:32
Copy link
Contributor

@rogerthatdev rogerthatdev left a comment

Choose a reason for hiding this comment

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

Please update the Google Cloud resource names to main

See cloud.google.com/docs/terraform/best-practices-for-terraform

bigquery/biglake/bigquery_create_object_table/main.tf Outdated Show resolved Hide resolved
@rogerthatdev rogerthatdev added the waiting-response Waiting for issue author to respond. label Sep 11, 2023
@rogerthatdev rogerthatdev self-assigned this Sep 11, 2023
@msampathkumar
Copy link
Contributor

@nevzheng - friendly reminder!

@nevzheng
Copy link
Contributor Author

I'm waiting for input from adhiggs@

@msampathkumar msampathkumar changed the title update: object table docs chore(bigquery): update object table docs Sep 25, 2023
@rogerthatdev rogerthatdev added waiting-for-codeowners and removed waiting-response Waiting for issue author to respond. labels Sep 29, 2023
@adhiggs
Copy link
Contributor

adhiggs commented Oct 3, 2023

Thank you for your patience. I'll get to this item this week.

Copy link
Contributor

@adhiggs adhiggs left a comment

Choose a reason for hiding this comment

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

If possible, we should adjust some of the items that are currently comment-locked. Thank you for your patience.

@nevzheng nevzheng force-pushed the update-object-table-sample branch from 137d6ea to 32cc5d9 Compare October 12, 2023 16:41
@nevzheng
Copy link
Contributor Author

@adhiggs If possible, we should adjust some of the items that are currently comment-locked. Thank you for your patience.

We can certainly do so. Let me know what the changes should be. line number + change.

@nevzheng nevzheng requested a review from adhiggs October 16, 2023 17:42
Copy link
Contributor

@msampathkumar msampathkumar left a comment

Choose a reason for hiding this comment

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

LGTM. Only update are here and CICD tests passed.

@msampathkumar msampathkumar enabled auto-merge (squash) October 20, 2023 22:19
*/

# [START bigquery_create_object_table]

# This queries the provider for project information.
data "google_project" "main" {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
data "google_project" "main" {}
data "google_project" "default" {}

resource "google_project_iam_member" "default" {
role = "roles/storage.objectViewer"
project = data.google_project.project.project_id
project = data.google_project.main.project_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
project = data.google_project.main.project_id
project = data.google_project.default.project_id

@msampathkumar
Copy link
Contributor

Almost there. As suggested by other reviewer, please check an update the resource name. Added code suggestions for the same.

@rogerthatdev rogerthatdev dismissed their stale review October 23, 2023 16:52

already reviewed by tf samples

@rogerthatdev rogerthatdev merged commit 3a70ce3 into terraform-google-modules:main Oct 23, 2023
5 checks passed
@nevzheng nevzheng deleted the update-object-table-sample branch October 23, 2023 18:16
msampathkumar added a commit to msampathkumar/terraform-docs-samples that referenced this pull request Dec 7, 2023
* update: object table docs

* update: fix comment, resource name

* update: integrate reviewer feedback

* update: integrate reviewer input

---------

Co-authored-by: Roger Martinez <[email protected]>
Co-authored-by: Sampath Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants