-
Notifications
You must be signed in to change notification settings - Fork 4k
[RecoveryServices.Backup] Removing name param from container #8088
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
Conversation
877229c
to
37f81de
Compare
39e4ceb
to
bcc4c4e
Compare
bcc4c4e
to
6895f65
Compare
@@ -30,7 +30,7 @@ public ContainerTests(Xunit.Abstractions.ITestOutputHelper output) | |||
XunitTracingInterceptor.AddToContext(_logger); | |||
} | |||
|
|||
[Fact] | |||
[Fact(Skip = "This workload is not supported anymore")] |
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.
Remove this test if it is not supported - same for all the other skips
@@ -30,7 +30,7 @@ public ItemTests(Xunit.Abstractions.ITestOutputHelper output) | |||
XunitTracingInterceptor.AddToContext(_logger); | |||
} | |||
|
|||
[Fact] | |||
[Fact(Skip = "This workload is not supported anymore")] |
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.
See comment above - remove skipped tests if this is no longer supported
...mands.RecoveryServices.Backup/Cmdlets/Container/GetAzureRmRecoveryServicesBackupContainer.cs
Show resolved
Hide resolved
33f92dd
to
6895f65
Compare
@@ -194,23 +194,12 @@ public AzureWorkloadProviderHelper(ServiceClientAdapter serviceClientAdapter) | |||
{ | |||
string vaultName = (string)providerData[CmdletModel.VaultParams.VaultName]; | |||
string vaultResourceGroupName = (string)providerData[CmdletModel.VaultParams.ResourceGroupName]; | |||
string name = (string)providerData[CmdletModel.ContainerParams.Name]; |
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.
See the file change above - remove this file 'set'
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.
Done
@@ -39,7 +39,7 @@ public void TestAzureSqlGetContainers() | |||
_logger, PsBackupProviderTypes.AzureSql, "Test-AzureSqlGetContainers"); | |||
} | |||
|
|||
[Fact] | |||
[Fact(Skip = "This workload is not supported anymore")] |
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.
You need to remove these unsupported scenario tests, not skip them
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.
Can I take this is next release? We have a whole cleanup to do related to this workload
@siddharth7 Note please make these changes in the next few hours, or this PR will not be merged an no breaking changes will be possible. |
Description
Checklist
CONTRIBUTING.md
platyPS
module