-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_spring_cloud_api_portal
- split to create/update functions
#24115
azurerm_spring_cloud_api_portal
- split to create/update functions
#24115
Conversation
ms-henglu
commented
Dec 5, 2023
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 @ms-henglu. Tests look fine, but there are some typos in the error messages.
existing, err := client.Get(ctx, id.ResourceGroup, id.SpringName, id.ApiPortalName) | ||
if err != nil { | ||
if !utils.ResponseWasNotFound(existing.Response) { | ||
return fmt.Errorf("retreiving %s: %+v", id, err) |
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 is misspelled multiple times, please fix all of them
return fmt.Errorf("retreiving %s: %+v", id, err) | |
return fmt.Errorf("retrieving %s: %+v", id, err) |
return fmt.Errorf("creating/updating %s: %+v", id, err) | ||
} | ||
|
||
if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { | ||
return fmt.Errorf("waiting for creation/update of %s: %+v", id, err) |
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.
The error messaged in the create function also need to be updated.
return fmt.Errorf("creating/updating %s: %+v", id, err) | |
} | |
if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { | |
return fmt.Errorf("waiting for creation/update of %s: %+v", id, err) | |
return fmt.Errorf("updating %s: %+v", id, err) | |
} | |
if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { | |
return fmt.Errorf("waiting for update of %s: %+v", id, err) |
Hi @stephybun , Thanks for the code review! I've updated this PR as suggested, please take another look. |
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 @ms-henglu LGTM 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |