-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 missing output fields to Certificate resource. #6322
Add missing output fields to Certificate resource. #6322
Conversation
Updated a couple of mistakes in fields descriptions. Changed type of Certificate.Managed.State from String to Enum.
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @shuyama1, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 6 files changed, 450 insertions(+), 38 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccPrivatecaCertificateAuthority_subordinateCaActivatedByFirstPartyIssuerOnCreation|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
These 3 test failures seems to be unrelated to the changes in this PR. Two of them have following error message:
and the third:
|
Thanks for making the changes! The failed tests are unrelated to this PR. Don't worry about those. Sorry for the noise. |
There are no issues with the type being String. I just saw a couple of Enums in the api.yaml and noticed that this one field is the only exception. I will revert it back to String I could also change other Enums to String in this file. In that case I assume that description field for such String type would contains only general description, without details about specific "enum" types. E.g.:
Would change to:
Is that correct? |
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.
Thanks for adding the fields and updating the documentation!
Just left some review comments and also it would nice if we could change all enum type to string. Thanks!
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 8 files changed, 305 insertions(+), 59 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccVPCAccessConnector_cloudrunVPCAccessConnectorExample|TestAccSqlUser_mysqlDisabled|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeRouterNat_withNatIpsAndDrainNatIps|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccActiveDirectoryDomain_update|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryPeering_activeDirectoryPeeringBasicExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Failed tests seem unrelated to these changes:
|
…rm#6322) Co-authored-by: Pawel Krawczyk <[email protected]>
Updated a couple of mistakes in fields descriptions. Changed type of
Certificate.Managed.State from String to Enum.
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)