cmdlet changes for creating and managing windows container web app #7191

exc3eed · 2018-09-11T01:39:58Z

Description

Azure/azure-powershell-cmdlet-review-pr#136
~~https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/145~~

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

exc3eed · 2018-09-12T00:00:50Z

Related design review PR https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/136

...ceManager/Websites/Commands.Websites/Cmdlets/DeploymentSlots/GetAzureWebAppSlotConfigName.cs

praries880 · 2018-09-12T22:13:52Z

src/ResourceManager/Websites/Commands.Websites/Commands.Websites.csproj

+    <Reference Include="Microsoft.Azure.Management.Websites, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Websites.2.0.0\lib\net452\Microsoft.Azure.Management.Websites.dll</HintPath>
+    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime.Azure, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">


remove this

take care of this.. these need to be removed

Lets address this in PR #7127

praries880 · 2018-09-12T22:14:00Z

src/ResourceManager/Websites/Commands.Websites/Commands.Websites.csproj

+    <Reference Include="Microsoft.Rest.ClientRuntime.Azure, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\..\packages\Microsoft.Rest.ClientRuntime.Azure.3.3.15\lib\net452\Microsoft.Rest.ClientRuntime.Azure.dll</HintPath>
+    </Reference>
+    <Reference Include="Newtonsoft.Json, Version=6.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL" />


remove this

same here.. this needs to be pulled out.

Should be fixed in #7127

Newtonsoft.Json and Microsoft.Rest.ClientRuntime.Azure need to be removed from this file. They are referenced from the common targets file (at the top of the csproj).

praries880 · 2018-09-12T22:16:19Z

src/ResourceManager/Websites/Commands.Websites/Models.WebApp/PSSite.cs

@@ -67,5 +68,8 @@ public PSSite(Site other)
        public string GitRemoteUri { get; set; }
        public string GitRemoteUsername { get; set; }
        public SecureString  GitRemotePassword { get; set; }
+
+        [CmdletParameterBreakingChange("SnapshotInfo", ChangeDescription = "This property is deprecated and will be removed in a future releases.")]


this needs to be applied on the cmdlet itself where PSSite is consumed. Adding it here will have no impact.

...er/Websites/Commands.Websites/Cmdlets/DeploymentSlots/GetAzureWebAppSlotPublishingProfile.cs

praries880 · 2018-09-12T22:21:43Z

...esourceManager/Websites/Commands.Websites/Cmdlets/WebApps/GetAzureWebAppPublishingProfile.cs

@@ -34,15 +34,19 @@ public class GetAzureWebAppPublishingProfileCmdlet : WebAppBaseCmdlet
        [ValidateSet("WebDeploy", "FileZilla3", "Ftp", IgnoreCase = true)]
        public string Format { get; set; }

+        [Parameter(Mandatory = false, HelpMessage = "Include the disaster recovery endpoints if true")]
+        public SwitchParameter IncludeDisasterRecoveryEndpoints { get; set; }


use a singular noun instead of the plural form.

Can you call the switch -IncludeDisasterRecoveryEndpointInformation or -IncludeDisasterRecoveryEndpointData

what I am saying is that PS recommends using singular nouns for params. Documentation here.

You can call this whatever internally but the param name should not be a plural.

This change is not part of this PR. It is part of #7127. Plural names for Switch parameters is fine as switch parameters cannot take values. So, plurality is irrelevant, other than to make the switch name make sense.

praries880 · 2018-09-13T00:09:06Z

@exc3eed @maddieclayton adding Do not merge in this PR because it depends on #7127 getting in first. The changes here are basically all changes in #7127 with only 4 cmdlets changes ove them

praries880

Limiting the scope of the review to just whats described in :"https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/136". The changes look good to me. Approved.

This can only be merged though after #7127 gets merged

exc3eed · 2018-09-13T01:09:44Z

@praries880 Thanks for reviewing this

MiYanni · 2018-09-14T17:06:22Z

@exc3eed Please pull the latest from the release-2018-09-17 branch.

src/ResourceManager/Websites/Commands.Websites/help/AzureRM.Websites.md

…et descriptions.

MiYanni · 2018-09-15T03:36:29Z

demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/1096/
ps-sign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/1333/
psnetsign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign/193/

@markcowl This PR is ready to merge once CI passes.

maddieclayton · 2018-09-15T06:37:28Z

@exc3eed It looks like merging Nick's PR caused some merge conflicts with your branch. I would merge them in Github, but looks like they are too complex to resolve in the web tool. Please pull from the upstream release-2018-09-17 branch to resolve merge conflicts.

…asfarok-container # Conflicts: # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestCreateNewWebAppBackup.json # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestCreateNewWebAppBackupPiping.json # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestEditAndGetWebAppBackupConfiguration.json # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestEditAndGetWebAppBackupConfigurationPiping.json # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestGetWebAppBackup.json # src/ResourceManager/Websites/Commands.Websites.Test/SessionRecords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppBackupRestoreTests/TestGetWebAppBackupList.json # src/ResourceManager/Websites/Commands.Websites/ChangeLog.md # src/ResourceManager/Websites/Commands.Websites/help/AzureRM.Websites.md

MiYanni · 2018-09-15T16:52:03Z

demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/1098/
ps-sign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/1335/
psnetsign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign/195/

…es prior state on the subscription to be able to run the test.

exc3eed changed the title ~~Asfarok container~~ cmdlet changes for creating and managing windows container web app Sep 11, 2018

azuresdkci1x added the needs-review label Sep 11, 2018

exc3eed requested review from jvano, nking-1, markcowl and panchagnula September 11, 2018 18:10

maddieclayton added Cmdlet Review Required ⚠️ Preview labels Sep 11, 2018

maddieclayton self-assigned this Sep 11, 2018

praries880 assigned praries880 and unassigned maddieclayton Sep 11, 2018

praries880 self-requested a review September 11, 2018 20:20

exc3eed added the Ignite label Sep 11, 2018

maddieclayton removed the Ignite label Sep 12, 2018

exc3eed added Ignite Cmdlet Review Required ⚠️ and removed Cmdlet Review Required ⚠️ Preview labels Sep 12, 2018

praries880 suggested changes Sep 12, 2018

View reviewed changes

maddieclayton added needs-revision and removed needs-review labels Sep 12, 2018

maddieclayton assigned exc3eed and unassigned praries880 Sep 12, 2018

praries880 added the Do Not Merge 🚫 label Sep 13, 2018

praries880 previously approved these changes Sep 13, 2018

View reviewed changes

maddieclayton assigned MiYanni Sep 13, 2018

maddieclayton removed the Ignite label Sep 13, 2018

MiYanni removed the Cmdlet Review Required ⚠️ label Sep 13, 2018

MiYanni mentioned this pull request Sep 13, 2018

New cmdlets to create/enter PSSession in remote container app. New cmdlet for getting container continuous deployment url #7231

Closed

8 tasks

maddieclayton changed the base branch from preview to release-2018-09-17 September 14, 2018 01:04

exc3eed dismissed praries880’s stale review via 55c88ad September 14, 2018 02:43

exc3eed force-pushed the asfarok-container branch from 4c34167 to 55c88ad Compare September 14, 2018 02:43

maddieclayton removed the Ignite label Sep 14, 2018

Cmdlets changes for managing web app containers

89359f9

exc3eed force-pushed the asfarok-container branch from 55c88ad to 89359f9 Compare September 15, 2018 00:37

MiYanni suggested changes Sep 15, 2018

View reviewed changes

src/ResourceManager/Websites/Commands.Websites/help/AzureRM.Websites.md Outdated Show resolved Hide resolved

MiYanni added 2 commits September 14, 2018 20:17

Fixed merge issue with cmdlet help file where it removed several cmdl…

cef8f2f

…et descriptions.

Fix changelog and psd1 release notes for release branch.

143aafc

MiYanni added Waiting for CI and removed Do Not Merge 🚫 needs-revision labels Sep 15, 2018

MiYanni previously approved these changes Sep 15, 2018

View reviewed changes

maddieclayton added Merge Conflicts and removed Waiting for CI labels Sep 15, 2018

MiYanni dismissed their stale review via 7686553 September 15, 2018 16:49

MiYanni added Waiting for CI and removed Merge Conflicts labels Sep 15, 2018

MiYanni previously approved these changes Sep 15, 2018

View reviewed changes

Re-recorded one of the tests. Skipping a Restore test since it requir…

55bf739

…es prior state on the subscription to be able to run the test.

MiYanni dismissed their stale review via 55bf739 September 15, 2018 18:51

MiYanni approved these changes Sep 15, 2018

View reviewed changes

MiYanni merged commit 379a4a2 into Azure:release-2018-09-17 Sep 16, 2018

cmdlet changes for creating and managing windows container web app #7191

cmdlet changes for creating and managing windows container web app #7191

Uh oh!

Conversation

exc3eed commented Sep 11, 2018 • edited by MiYanni Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

exc3eed commented Sep 12, 2018

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praries880 commented Sep 13, 2018

Uh oh!

praries880 left a comment

Choose a reason for hiding this comment

Uh oh!

exc3eed commented Sep 13, 2018

Uh oh!

MiYanni commented Sep 14, 2018

Uh oh!

Uh oh!

MiYanni commented Sep 15, 2018

Uh oh!

maddieclayton commented Sep 15, 2018

Uh oh!

MiYanni commented Sep 15, 2018

Uh oh!

Uh oh!

exc3eed commented Sep 11, 2018 •

edited by MiYanni

Loading