Skip to content

[FrontDoor Management Client] Update WAF and NetworkExperiment SDKs #8616

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
isra-fel merged 8 commits into from
Nov 8, 2019
Merged

[FrontDoor Management Client] Update WAF and NetworkExperiment SDKs #8616

isra-fel merged 8 commits into from
Nov 8, 2019

Conversation

@eschwabe
Copy link
Contributor

@eschwabe eschwabe commented Nov 5, 2019 •
edited
Loading

@eschwabe eschwabe requested a review from erich-wang as a code owner November 5, 2019 00:46
@eschwabe
Copy link
Contributor Author

eschwabe commented Nov 5, 2019

FYI. @jessicl-ms, @Diego-Perez-Botero, @digavino

Diego-Perez-Botero
Diego-Perez-Botero reviewed Nov 5, 2019
View reviewed changes
...frontdoor/Microsoft.Azure.Management.FrontDoor/tests/ScenarioTests/NetworkExperimentTests.cs
public class NetworkExperimentTests
{
[Fact]
public void NetworkExperimentCRUDTest()
Copy link

@Diego-Perez-Botero Diego-Perez-Botero Nov 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

NetworkExperimentCRUDTest [](start = 20, length = 25)

Thanks for writing this test!! :) #Resolved

Diego-Perez-Botero
Diego-Perez-Botero approved these changes Nov 5, 2019
View reviewed changes
Copy link

@Diego-Perez-Botero Diego-Perez-Botero left a comment

Choose a reason for hiding this comment

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

:shipit:

@isra-fel
Copy link
Member

isra-fel commented Nov 6, 2019

Hi @eschwabe , could you

  • Add a link to the swagger spec review PR(s) in the description
  • Include the .txt file generated when you run generate.ps1(details)
  • Use version 2.0.0 since there are breaking changes
  • Update AssemblyInfo.cs as well

Thanks

@isra-fel isra-fel added Mgmt This issue is related to a management package. needs-review and removed needs-review labels Nov 6, 2019
@Diego-Perez-Botero
Copy link

Diego-Perez-Botero commented Nov 6, 2019

@eschwabe - Swagger spec review PR for NetworkExperiment is: Azure/azure-rest-api-specs#7370

@eschwabe eschwabe removed their assignment Nov 6, 2019
@eschwabe
Copy link
Contributor Author

eschwabe commented Nov 6, 2019

@isra-fel Addressed your comments. Could you please review the latest iteration? Thanks!

@isra-fel
Copy link
Member

isra-fel commented Nov 7, 2019

@isra-fel Addressed your comments. Could you please review the latest iteration? Thanks!

The metadata txt was not in the right place, should be eng/mgmt/mgmtmetadata/.
Please follow this guide to update your build tools then regenreate.

Other changes are good to me. Thanks.

@eschwabe
Copy link
Contributor Author

eschwabe commented Nov 7, 2019 •
edited
Loading

@isra-fel Addressed your comments. Could you please review the latest iteration? Thanks!

The metadata txt was not in the right place, should be eng/mgmt/mgmtmetadata/.
Please follow this guide to update your build tools then regenreate.

Other changes are good to me. Thanks.

@isra-fel I had an old version of the powershell module installed. The generated code should be correct now. Thanks!

@isra-fel isra-fel merged commit c62ad27 into Azure:master Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Diego-Perez-Botero Diego-Perez-Botero Diego-Perez-Botero approved these changes

@erich-wang erich-wang Awaiting requested review from erich-wang

+1 more reviewer

@isra-fel isra-fel isra-fel approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Mgmt This issue is related to a management package.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eschwabe @isra-fel @Diego-Perez-Botero