Skip to content

PR : Update to accomdate SDK changes with VNET reference #12117

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 12, 2020
Merged

PR : Update to accomdate SDK changes with VNET reference #12117

merged 4 commits into from
Jun 12, 2020

Conversation

@aegal
Copy link
Contributor

@aegal aegal commented Jun 11, 2020 •
edited
Loading

Description

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

@aegal aegal requested a review from anton-evseev as a code owner June 11, 2020 00:05
@adxsdkps
Copy link
Collaborator

adxsdkps commented Jun 11, 2020

Can one of the admins verify this patch?

@aegal aegal changed the title Update to accomdate SDK changes with VNET reference PR : Update to accomdate SDK changes with VNET reference Jun 11, 2020
@aegal aegal requested review from gmainar and ninzavivek June 11, 2020 00:13
@VeryEarly VeryEarly self-assigned this Jun 11, 2020
@VeryEarly
Copy link
Collaborator

VeryEarly commented Jun 11, 2020

Hi @aegal ,

Can you try pull latest changes from network-may branch, this should fix test failures

gmainar
gmainar reviewed Jun 11, 2020
View reviewed changes
src/Network/Network/Network.format.ps1xml Outdated
<ListItem>
<Label>NetworkInterfaceIpConfiguration</Label>
<Label>NetworkInterfaceIpConfigurationId</Label>
<PropertyName>NetworkInterfaceIpConfigurationText</PropertyName>
Copy link
Contributor

@gmainar gmainar Jun 11, 2020

Choose a reason for hiding this comment

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

shouldn't we change this to NetworkInterfaceIpConfigurationIdText?

Copy link
Contributor Author

@aegal aegal Jun 11, 2020

Choose a reason for hiding this comment

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

No, that label is what is printed on the powershell console. I think it would read kind of weird to add 'text' to the output console

Copy link
Contributor Author

@aegal aegal Jun 11, 2020

Choose a reason for hiding this comment

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

Same as the other comment.

src/Network/Network/Network.format.ps1xml Outdated
<ListItem>
<Label>VirtualNetwork</Label>
<Label>VirtualNetworkId</Label>
<PropertyName>VirtualNetworkChildResourceText</PropertyName>
Copy link
Contributor

@gmainar gmainar Jun 11, 2020

Choose a reason for hiding this comment

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

shouldn't this be VirtualNetworkIdText

Copy link
Contributor Author

@aegal aegal Jun 11, 2020

Choose a reason for hiding this comment

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

No, that label is what is printed on the powershell console. I think it would read kind of weird to add 'text' to the output console

Copy link
Contributor Author

@aegal aegal Jun 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ohh sorry, you referring to VirtualNetworkChildResourceText property. Yeah that property name it isn't visible but I can update that for code readability.

@VeryEarly
Copy link
Collaborator

VeryEarly commented Jun 12, 2020

Hi @aegal ,

Can you please add changelog?

@aegal aegal merged commit f6b8af1 into network-may Jun 12, 2020
@VeryEarly
Copy link
Collaborator

VeryEarly commented Jun 13, 2020

@aegal ,

Although network-may is a network release branch, it will be merged into master eventually and it's impossible for us to review with so many commits altogether.

Please DO NOT merge by yourself without approval from Azure-PowerShell team.

@wyunchi-ms wyunchi-ms deleted the updateVnetReference branch January 10, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@gmainar gmainar gmainar approved these changes

@anton-evseev anton-evseev anton-evseev approved these changes

@ninzavivek ninzavivek ninzavivek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@VeryEarly VeryEarly

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@aegal @adxsdkps @VeryEarly @anton-evseev @gmainar @ninzavivek