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_agent #21273

Merged
merged 12 commits into from
Apr 20, 2023
Merged

New Resource : azurerm_storage_mover_agent #21273

merged 12 commits into from
Apr 20, 2023

Conversation

sinbai
Copy link
Contributor

@sinbai sinbai commented Apr 4, 2023 •
edited
Loading

tombuildsstuff
tombuildsstuff suggested changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hi @sinbai

Thanks for this PR - I've taken a look through and left some comments inline, if we can fix those up then we should be able to take another look here.

Thanks!

internal/services/storagemover/storage_mover_agent_resource.go Outdated Show resolved Hide resolved
internal/services/storagemover/storage_mover_agent_resource.go Outdated Show resolved Hide resolved
internal/services/storagemover/storage_mover_agent_resource.go Outdated
StorageMoverId: storagemovers.NewStorageMoverID(id.SubscriptionId, id.ResourceGroupName, id.StorageMoverName).ID(),
}

properties := &model.Properties
Copy link
Contributor

Choose a reason for hiding this comment

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

.. why are we de-referencing this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code has been updated.

internal/services/storagemover/storage_mover_agent_resource_test.go Outdated
Comment on lines 102 to 104
provider "azurerm" {
features {}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

the provider block should be in each test, not the template - to allow for overriding it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code has been updated.

internal/services/storagemover/storage_mover_agent_resource_test.go Outdated Show resolved Hide resolved
website/docs/r/storage_mover_agent.html.markdown Outdated Show resolved Hide resolved
website/docs/r/storage_mover_agent.html.markdown Outdated Show resolved Hide resolved
website/docs/r/storage_mover_agent.html.markdown Outdated Show resolved Hide resolved
website/docs/r/storage_mover_agent.html.markdown Outdated Show resolved Hide resolved
website/docs/r/storage_mover_agent.html.markdown Outdated Show resolved Hide resolved
@sinbai sinbai force-pushed the storagemover/new_resource_storagemover_agent branch from 99a3499 to e77aef1 Compare April 7, 2023 06:51
@sinbai sinbai force-pushed the storagemover/new_resource_storagemover_agent branch from 183cfc6 to c2b33df Compare April 13, 2023 09:13
@sinbai sinbai force-pushed the storagemover/new_resource_storagemover_agent branch from c2b33df to bda7d85 Compare April 13, 2023 09:34
@sinbai
Copy link
Contributor Author

sinbai commented Apr 14, 2023 •
edited
Loading

hi @sinbai

Thanks for this PR - I've taken a look through and left some comments inline, if we can fix those up then we should be able to take another look here.

Thanks!

Hi @tombuildsstuff

Thanks for all your detailed feedback. I have fixed/replied. Could you please take another look?

Test results:
image

stephybun
stephybun reviewed Apr 19, 2023
View reviewed changes
Copy link
Member

@stephybun stephybun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for making those updates @sinbai. I have two suggestions and this needs to be rebased but otherwise this looks like it's good to go!

internal/services/storagemover/storage_mover_agent_resource.go Outdated Show resolved Hide resolved
internal/services/storagemover/storage_mover_agent_resource.go Outdated Show resolved Hide resolved
@sinbai sinbai force-pushed the storagemover/new_resource_storagemover_agent branch from 414b5f4 to c039e94 Compare April 20, 2023 05:46
@sinbai
Copy link
Contributor Author

sinbai commented Apr 20, 2023

Thanks for making those updates @sinbai. I have two suggestions and this needs to be rebased but otherwise this looks like it's good to go!

@stephybun Thanks for your feedback. The code has been updated. Could you please take another look?
Test results:
image

stephybun
stephybun approved these changes Apr 20, 2023
View reviewed changes
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Good stuff, thanks @sinbai! LGTM 🐲

@stephybun stephybun merged commit 1656a27 into hashicorp:main Apr 20, 2023
@github-actions github-actions bot added this to the v3.53.0 milestone Apr 20, 2023
stephybun added a commit that referenced this pull request Apr 20, 2023
@stephybun
Update for #21273
b64c146
@github-actions
Copy link

This functionality has been released in v3.53.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_storagemover_agent branch March 28, 2024 02:51
@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

@tombuildsstuff tombuildsstuff tombuildsstuff requested changes

@stephybun stephybun stephybun approved these changes

Assignees
No one assigned
Labels
dependencies documentation new-resource service/storagemover size/XL
Projects
None yet
Milestone
v3.53.0
Development

Successfully merging this pull request may close these issues.
3 participants
@sinbai @tombuildsstuff @stephybun