-
Notifications
You must be signed in to change notification settings - Fork 4k
Adding new parameters for CustomIpPrefix #15333
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
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 change log for your change.
src/Network/Network/BYOIP/CustomIpPrefix/UpdateAzureCustomIpPrefixCommand.cs
Show resolved
Hide resolved
public SwitchParameter Decommission { get; set; } | ||
|
||
[Parameter(Mandatory = false, HelpMessage = "Run cmdlet in the background")] | ||
public SwitchParameter Provision { get; set; } |
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.
- I recommend using bool as the type of Provision so that we needn't two parameters for one function.
- Please update the help message.
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.
type would change behavior of our command because we need to use like
'''-Provision $true''' right?
Hi @gitlwh , How about Provision and Deprovision? |
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.
Hi @gitlwh , How about Provision and Deprovision?
What do you mean by that? I think they should have same behavior as commission and decommission. Only one of the four should exist.
src/Network/Network/BYOIP/CustomIpPrefix/UpdateAzureCustomIpPrefixCommand.cs
Show resolved
Hide resolved
public SwitchParameter Decommission { get; set; } | ||
|
||
[Parameter(Mandatory = false, HelpMessage = "Run cmdlet in the background")] | ||
public SwitchParameter Provision { get; set; } |
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.
type would change behavior of our command because we need to use like
'''-Provision $true''' right?
How to check this logic? It's in backend side?
Yes, I prefer this behavior because we can avoid customer using Provision and Deprovision at same time |
If you read my code, my implementation is And if we change the behavior of command it would be a break change right? |
|
src/Network/Network/ChangeLog.md
Outdated
@@ -48,6 +48,9 @@ | |||
- `Update-AzVirtualHub` | |||
* Updated cmdlets to expose two read-only properties of client certificate. | |||
- `Get-AzApplicationGatewayTrustedClientCertificate` | |||
* Updated cmdlets to add properties for new BYOIP features. | |||
- `New-AzCustomIpPrefix` |
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.
No, The original way is more simple for cx to use and we don't have a plan to change this bahavior. |
…hell into weihl/networkmarch
Added one more parameter for update operation CIDR |
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