-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 Firebase AppleApp Data Source #6876
Conversation
…ion so they show up in docs.
Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
…pdate_test.go.erb Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
@roaks3 All tests pass and this PR is ready now. |
@roaks3 Ping! |
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.
Code looks good, only a few minor comments and then this should be good to go.
@@ -12,7 +12,7 @@ func dataSourceGoogleFirebaseWebApp() *schema.Resource { | |||
dsSchema := datasourceSchemaFromResourceSchema(resourceFirebaseWebApp().Schema) | |||
|
|||
// Set 'Required' schema elements | |||
addRequiredFieldsToSchema(dsSchema, "app_id") |
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.
Could you remove the whitespace changes? It doesn't look like this file has any changes of interest.
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.
@mraouffouad just following up because I saw you made updates, but the whitespace changes have not been addressed yet. For example, this file should have no changes in the final diff.
@@ -216,7 +216,7 @@ func Provider() *schema.Provider { | |||
"google_compute_backend_service": dataSourceGoogleComputeBackendService(), | |||
"google_compute_backend_bucket": dataSourceGoogleComputeBackendBucket(), | |||
"google_compute_default_service_account": dataSourceGoogleComputeDefaultServiceAccount(), | |||
"google_compute_disk": dataSourceGoogleComputeDisk(), | |||
"google_compute_disk": dataSourceGoogleComputeDisk(), |
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.
Same comment around whitespace changes: could you revert them here and on the other lines in this file?
|
||
* `app_id` - | ||
(Required) | ||
The app_ip of name of the Firebase iosApp. |
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.
Looks like we want app_id
instead of app_ip
. I think this may be a mistake in the other Firebase datasources as well, so feel free to make those updates in this same PR if you'd like.
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 60 insertions(+), 1 deletion(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 60 insertions(+), 1 deletion(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccLoggingBucketConfigProject_cmekSettings |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 60 insertions(+), 1 deletion(-)) |
1 similar comment
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 60 insertions(+), 1 deletion(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_withInvalidGatewayApiConfigChannel |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccContainerCluster_withInvalidGatewayApiConfigChannel |
@roaks3 Thanks for reviewing. Please merge. |
A new datasource for Firebase AppleApp.
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)