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.

Sign up for GitHub

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

Jump to bottom

New Resource : azurerm_storage_mover_project #21477

Merged
merged 11 commits into from
Apr 21, 2023
Merged

New Resource : azurerm_storage_mover_project #21477

merged 11 commits into from
Apr 21, 2023

Conversation

sinbai
Copy link
Contributor

@sinbai sinbai commented Apr 19, 2023 •
edited
Loading

@sinbai sinbai force-pushed the storagemover/new_resource_project branch from a8c25e8 to 07f330e Compare April 19, 2023 06:23
@sinbai sinbai force-pushed the storagemover/new_resource_project branch from 07f330e to 2dde72b Compare April 20, 2023 08:50
mbfrahry
mbfrahry requested changes Apr 20, 2023
View reviewed changes
Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

Hey @sinbai, this PR looks good! Just need another nil check and a simplified way to set a variable. Once those and the merge conflicts are fixed, we'll be good to go!

internal/services/storagemover/storage_mover_project_resource.go Outdated
}

if metadata.ResourceData.HasChange("description") {
properties.Properties.Description = &model.Description
Copy link
Member

Choose a reason for hiding this comment

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

properties.Properties could be nil so we need to check that it isn't before trying to set properties.Properties.Description.

properties.Properties.Description = &model.Description

to

properties.Properties.Description = pointer.To(model.Description)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

internal/services/storagemover/storage_mover_project_resource.go Outdated

if resp.Model != nil {
if properties := resp.Model.Properties; properties != nil {
if properties.Description != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This if block can be replaced with

Suggested change
if properties.Description != nil {
state.Description = pointer.From(properties.Description)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@sinbai sinbai force-pushed the storagemover/new_resource_project branch from 2dde72b to 0b87a5a Compare April 21, 2023 02:31
@sinbai
Copy link
Contributor Author

sinbai commented Apr 21, 2023

Hey @sinbai, this PR looks good! Just need another nil check and a simplified way to set a variable. Once those and the merge conflicts are fixed, we'll be good to go!

Hi @mbfrahry, thanks for your feedback. I have updated the code. Could you please take another look?

Test results:
image

mbfrahry
mbfrahry approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for those changes @sinbai

@mbfrahry mbfrahry added this to the v3.54.0 milestone Apr 21, 2023
@mbfrahry mbfrahry merged commit c93f53c into hashicorp:main Apr 21, 2023
mbfrahry added a commit that referenced this pull request Apr 21, 2023
@mbfrahry
Update CHANGELOG.md for #21477
40f6f0f
sinbai added a commit to sinbai/terraform-provider-azurerm that referenced this pull request Apr 24, 2023
@sinbai
New Resource : azurerm_storage_mover_project (hashicorp#21477)
f6ea646
sinbai pushed a commit to sinbai/terraform-provider-azurerm that referenced this pull request Apr 24, 2023
@mbfrahry @sinbai
Update CHANGELOG.md for hashicorp#21477
3b3fa1f
@github-actions
Copy link

This functionality has been released in v3.54.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@sinbai sinbai deleted the storagemover/new_resource_project branch March 28, 2024 02:50
@github-actions GitHub Actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mbfrahry mbfrahry mbfrahry approved these changes

Assignees
No one assigned
Labels
dependencies documentation size/XL
Projects
None yet
Milestone
v3.54.0
Development

Successfully merging this pull request may close these issues.
3 participants
@sinbai @mbfrahry @katbyte