-
Notifications
You must be signed in to change notification settings - Fork 4k
Adding -AzureStorageAccounts parameter to Set-AzureRmWebApp and Set-AzureRmWebAppSlot #7885
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
…app and Linux webapps
Disabled tests have now been enabled. |
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.
@vinisoto a few minor comments to take a look at, otherwise LGTM
@@ -0,0 +1,52 @@ | |||
using Microsoft.Azure.Commands.WebApps.Models; |
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.
@vinisoto please add a license header to this file
@@ -0,0 +1,24 @@ | |||
using Microsoft.Azure.Management.WebSites.Models; |
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.
@vinisoto please add a license header to this file
|
||
namespace Microsoft.Azure.Commands.WebApps.Cmdlets.WebApps | ||
{ | ||
[Cmdlet("New", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "WebAppAzureStoragePath", SupportsShouldProcess = true), OutputType(typeof(WebAppAzureStoragePath))] |
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.
@vinisoto instead of "New", please use VerbsCommon.New
|
||
namespace Microsoft.Azure.Commands.WebApps.Cmdlets.WebApps | ||
{ | ||
[Cmdlet("New", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "WebAppAzureStoragePath", SupportsShouldProcess = true), OutputType(typeof(WebAppAzureStoragePath))] |
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.
@vinisoto as a note, it's not necessary to support ShouldProcess
in this scenario since you are creating an in-memory object, but it doesn't hurt to have it supported 😀
Thanks @cormacpayne. I addressed the requested changes. |
Description
Re-submitting #7295
Design review has been approved: Azure/azure-powershell-cmdlet-review-pr#166
Adding -AzureStorageAccounts parameter to Set-AzureRmWebApp and Set-AzureRmWebAppSlot to support "Bring your own storage" to Container-based web apps
Checklist
CONTRIBUTING.md
platyPS
module