-
Notifications
You must be signed in to change notification settings - Fork 4k
Adding cmdltets to support crud operations for new Route Table child resource of VirtualHub #10317
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
Adding cmdltets to support crud operations for new Route Table child resource of VirtualHub #10317
Conversation
Can one of the admins verify this patch? |
PS Design Review PR - https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/397 |
The changes regarding support for EnableInternetSecurity for VirtualHubVnetConnection and VpnConnection were committed to a private-preview version in this PR - #7261. I am just picking the changes we need here. |
I have created the PR by building the sdk locally, will revert the SDK changes when needed, |
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.
Please add test(s) for these changes
I am working on adding the tests locally and will update the PR by tomorrow. Meanwhile it would be great if you could review the other code changes if possible. |
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.
There's a bit of inconsistency with EnableInternetSecurity
: in some cmdlets it's a switch, in some - nullable bool. I get that it's done so in Set the value wouldn't be reset, but I think having two different types for one parameter would be confusing
Please also re-generate help files
src/Network/Network/Cortex/HubVnetConnection/UpdateAzureRmHubVirtualNetworkConnectionCommand.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/Cortex/HubVnetConnection/UpdateAzureRmHubVirtualNetworkConnectionCommand.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/Cortex/VirtualHubRouteTable/VirtualHubRouteTableBaseCmdlet.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/Cortex/VirtualHubRouteTable/RemoveAzureRmVirtualHubRouteTableCommand.cs
Outdated
Show resolved
Hide resolved
src/Network/Network/Cortex/VirtualHubRouteTable/RemoveAzureRmVirtualHubRouteTableCommand.cs
Outdated
Show resolved
Hide resolved
We've checked in test recordings with API version 2019-09-01 into |
src/Network/Network/Cortex/VirtualHub/NewAzureRmVirtualHubRouteCommand.cs
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Description
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added