Skip to content
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

added global reach flag in New-AzureExpressRouteCircuit cmdlet #6845

Closed

Conversation

syfarogh
Copy link
Contributor

@syfarogh syfarogh commented Aug 3, 2018

Description

added global reach flag in New-AzureExpressRouteCircuit cmdlet to toggle Global reach feature on a circuit.

Checklist

Mandatory = false,
ValueFromPipelineByPropertyName = true)]
[ValidateNotNullOrEmpty]
public bool AllowGlobalReach { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean parameters are discouraged. You should use a SwitchParameter instead.

@@ -103,6 +103,11 @@ public class NewAzureExpressRouteCircuitCommand : ExpressRouteCircuitBaseCmdlet
[ValidateNotNullOrEmpty]
public bool? AllowClassicOperations { get; set; }

[Parameter(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should ensure the new parameter is covered in one of your tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated an existing test to test this flag.

@markcowl
Copy link
Member

markcowl commented Aug 6, 2018

@syfarogh Code complete is today (8/6) Please make requested changes by EOD if you would like this PR in this release.

@syfarogh
Copy link
Contributor Author

syfarogh commented Aug 7, 2018

The test is failing because it requires SDK generated from swagger network-august-release.

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests failing - need to be re-recorded

@@ -154,7 +154,7 @@ function Test-ExpressRouteCircuitCRUD
$resourceGroup = New-AzureRmResourceGroup -Name $rgname -Location $rglocation

# Create the ExpressRouteCircuit
$circuit = New-AzureRmExpressRouteCircuit -Name $circuitName -Location $location -ResourceGroupName $rgname -SkuTier Standard -SkuFamily MeteredData -ServiceProviderName "equinix" -PeeringLocation "Silicon Valley" -BandwidthInMbps 500;
$circuit = New-AzureRmExpressRouteCircuit -Name $circuitName -Location $location -ResourceGroupName $rgname -SkuTier Standard -SkuFamily MeteredData -ServiceProviderName "equinix" -PeeringLocation "Silicon Valley" -BandwidthInMbps 500 -AllowGlobalReach;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you will need to re-record tests for this

@maddieclayton
Copy link
Contributor

@syfarogh Ping

@maddieclayton
Copy link
Contributor

Closing per offline conversation that @syfarogh will open a new PR targeting a different branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants