Skip to content

[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

Merged
merged 5 commits into from
Dec 13, 2018

Conversation

siddharth7
Copy link
Contributor

Description

Checklist

@siddharth7 siddharth7 changed the base branch from preview to master December 11, 2018 10:25
@siddharth7 siddharth7 force-pushed the rs_breaking branch 2 times, most recently from 877229c to 37f81de Compare December 11, 2018 10:34
@siddharth7 siddharth7 force-pushed the rs_breaking branch 2 times, most recently from 39e4ceb to bcc4c4e Compare December 11, 2018 12:26
@siddharth7 siddharth7 changed the title [RecoveryServices.Backup] Removing name param and set-vault context [RecoveryServices.Backup] Removing name param from container Dec 11, 2018
@maddieclayton maddieclayton added the Breaking Change Release This PR contains breaking change label Dec 12, 2018
@@ -30,7 +30,7 @@ public ContainerTests(Xunit.Abstractions.ITestOutputHelper output)
XunitTracingInterceptor.AddToContext(_logger);
}

[Fact]
[Fact(Skip = "This workload is not supported anymore")]
Copy link
Member

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")]
Copy link
Member

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

@@ -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];
Copy link
Member

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'

Copy link
Contributor Author

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")]
Copy link
Member

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

Copy link
Contributor Author

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

@markcowl markcowl changed the base branch from master to release-2018-12-18 December 13, 2018 08:31
@markcowl
Copy link
Member

@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.

@markcowl markcowl merged commit 37292b4 into Azure:release-2018-12-18 Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Release This PR contains breaking change Sprint Candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants