Skip to content

Updated Connection monitor endpoint cmdlet #12852

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

Closed
wants to merge 4 commits into from
Closed

Updated Connection monitor endpoint cmdlet #12852

wants to merge 4 commits into from

Conversation

irrogozh
Copy link
Member

@irrogozh irrogozh commented Sep 4, 2020

Description

Same pr was already reviewed in network-june branch
#12851 (comment)

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • 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:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@msJinLei
Copy link
Contributor

msJinLei commented Sep 8, 2020

@msJinLei
Copy link
Contributor

msJinLei commented Sep 9, 2020

cmdlet review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/733
PR duplicate: #12851

@irrogozh Can this PR be closed since it is duplicate with #12851

@irrogozh
Copy link
Member Author

irrogozh commented Sep 9, 2020

@msJinLei , no, actually this pr should be the main one. We can close pr in network-june branch

[Parameter(
Mandatory = true,
HelpMessage = "The type of the connection monitor endpoint. Supported types are AzureVM, AzureVNet, AzureSubnet, ExternalAddress, MMAWorkspaceMachine, MMAWorkspaceNetwork.")]
[ValidateNotNullOrEmpty]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add mandatory parameter is a breaking change.

  • add breaking change notification to master first
  • release your breaking change in major release in Oct.

Copy link
Member Author

Choose a reason for hiding this comment

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

@msJinLei , we discussed that it's not necessary, because the feature is in Public preview:( Not in GA. We need it to be in this release for Ignite.

Copy link
Contributor

@msJinLei msJinLei Sep 10, 2020

Choose a reason for hiding this comment

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

@msJinLei , we discussed that it's not necessary, because the feature is in Public preview:( Not in GA. We need it to be in this release for Ignite.

@irrogozh

The PR is targeting to master branch, in which breaking change is not allowed.

  • If you want to release these breaking change features. we can release it with a OOB release rather than next regular release. You need to create a preview branch (or with an existing branch) to include this PR and request for a OOB release after next regular release.
  • If you want to merge these commits to master, you need wait for the major release.

@msJinLei msJinLei self-assigned this Sep 14, 2020
@msJinLei
Copy link
Contributor

closed since duplicate with #12943

@msJinLei msJinLei closed this Sep 15, 2020
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