Skip to content

[SQL] Deprecate LTR retention vault cmdlets#7022

Closed
adeal wants to merge 4 commits intoAzure:AzureRM.Sql.STRfrom
adeal:AzureRM.Sql.STR
Closed

[SQL] Deprecate LTR retention vault cmdlets#7022
adeal wants to merge 4 commits intoAzure:AzureRM.Sql.STRfrom
adeal:AzureRM.Sql.STR

Conversation

@adeal
Copy link
Copy Markdown
Contributor

@adeal adeal commented Aug 22, 2018 •
edited
Loading

Description

This change deprecates two LTR cmdlets:

  • Get-AzureRmSqlServerBackupLongTermRetentionVault
  • Set-AzureRmSqlServerBackupLongTermRetentionVault

These cmdlets are no longer supported and a replacement is not needed.

Checklist

@adeal adeal requested a review from jaredmoo August 22, 2018 23:09
@adeal adeal requested a review from pixia August 22, 2018 23:23
@maddieclayton maddieclayton changed the base branch from AzureRM.Sql.STR to preview August 23, 2018 19:58
@maddieclayton maddieclayton changed the base branch from preview to AzureRM.Sql.STR August 23, 2018 19:58
@maddieclayton
Copy link
Copy Markdown
Contributor

maddieclayton commented Aug 23, 2018

@dealaus This looks great! Two comments though:

  1. Can you target this at preview so that we get the deprecation in the next stable release?
  2. Can you add back in the test, but add this around it to ensure that it only runs on Desktop?
#If !NETSTANDARD
        [Fact]
        [Trait(Category.AcceptanceType, Category.CheckIn)]
        public void TestServerBackupLongTermRetentionVault()
        {
            // TODO Rewrite SQL backup tests to be recordable
            // TODO https://github.com/Azure/azure-powershell/issues/4155
            if (TestMockSupport.RunningMocked)
            {
                RunPowerShellTest("Test-ServerBackupLongTermRetentionVault");
            }
        }
#Endif

@adeal adeal changed the base branch from AzureRM.Sql.STR to preview August 23, 2018 20:18
@adeal adeal changed the base branch from preview to AzureRM.Sql.STR August 23, 2018 20:19
@adeal
Copy link
Copy Markdown
Contributor Author

adeal commented Aug 23, 2018

I figured there was a why to not include the test for just netcore. Thanks!

Changing the base branch has quite the merge conflict. I think it'll be quicker to just abandon this and create a new PR targeting preview.

@adeal adeal changed the base branch from AzureRM.Sql.STR to preview August 23, 2018 20:21
@adeal adeal changed the base branch from preview to AzureRM.Sql.STR August 23, 2018 20:23
@adeal adeal closed this Aug 23, 2018
@adeal adeal mentioned this pull request Aug 23, 2018
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@jaredmoo jaredmoo jaredmoo approved these changes

@pixia pixia pixia approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adeal adeal

Labels

needs-revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@adeal @maddieclayton @jaredmoo @pixia @azuresdkci1x