[RecoveryServices.Backup] Added cmdlets to enable or disable SoftdeleteFeature in a vault #10571
Conversation
|
Can one of the admins verify this patch? |
| { | ||
| [Parameter(Mandatory = true, ValueFromPipeline = false)] | ||
| [ValidateNotNullOrEmpty] | ||
| public VaultSoftDeleteFeatureState SoftDeleteFeatureState { get; set; } |
There was a problem hiding this comment.
It will be better to set the type of SoftDeleteFeatureState as string and set the value range to Enabled and Disabled. You can find example here
There was a problem hiding this comment.
@wyunchi-ms : I have seen the example but I am still not convinced/clear. I want to set the feature state with this command and hence I want to "enable/disable" it. Later when I get the value, I am expecting the feature to be either "Enabled/Disabled". May I know what is incorrect in this assumption?
There was a problem hiding this comment.
Both Enable and Enabled are fine. You can choose by youself. But I suggest to use ValidateSet to make sure user input the right value that you expect.
There was a problem hiding this comment.
The type of SoftDeleteFeatureState should be string and VaultSoftDeleteFeatureState is no need any more. You can use ValidateSet to ensure the user input the right value, and there is no need to add a new type for this.
|
|
||
| BackupResourceVaultConfigResource param = new BackupResourceVaultConfigResource(); | ||
| param.Properties = new BackupResourceVaultConfig(); | ||
| param.Properties.SoftDeleteFeatureState = SoftDeleteFeatureState.ToString() + "d"; |
There was a problem hiding this comment.
After set the type to string, "d" is no need
There was a problem hiding this comment.
@wyunchi-ms as per the cmdlet review https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/437 it was decided to accept enable/disable as values for set command.
Adding @pvrk for reference
wyunchi-ms
dismissed
their stale review
The type of SoftDeleteFeatureState need more discus
Description
Checklist
CONTRIBUTING.mdChangeLog.mdfile(s) has been updated:ChangeLog.mdfile can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md## Upcoming Releaseheader -- no new version header should be added